Stream read and write methods

C programming, build, interpreter/VM.
Target audience: MicroPython Developers.
Post Reply
blmorris
Posts: 348
Joined: Fri May 02, 2014 3:43 pm
Location: Massachusetts, USA

Stream read and write methods

Post by blmorris » Mon Nov 02, 2015 7:09 pm

I'm going over the code for reading from and writing to a file / stream in my I2S driver, trying to clean it up and make it more consistent. While writing the code, I implemented file reading / I2S playback first; the following snippet was adapted from very similar code in 'py/stream.c':

Code: Select all

    pyb_i2s_obj_t *self = pos_args[0];
    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
    mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
    
    mp_obj_t stream = args[0].u_obj;
    mp_obj_type_t *type = mp_obj_get_type(stream);
    if (type->stream_p == NULL || type->stream_p->read == NULL) {
        nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_ValueError, "Object type %s not a readable stream",
                                                mp_obj_get_type_str(stream)));
    }
...
    self->out_sz = self->dstream_tx->type->stream_p->read(self->dstream_tx, &self->audiobuf_tx[self->pp_ptr], buf_sz, &error);

    if (self->out_sz == MP_STREAM_ERROR) {
        if (mp_is_nonblocking_error(error)) {
            // nonblocking error behavior copied from py/stream.c: stream_read()
            // see https://docs.python.org/3.4/library/io.html#io.RawIOBase.read
            return mp_const_none;
        }
        nlr_raise(mp_obj_new_exception_arg1(&mp_type_OSError, MP_OBJ_NEW_SMALL_INT(error)));
    }
However, for writing to a stream, py/stream.c provides the following public function:

Code: Select all

mp_obj_t mp_stream_write(mp_obj_t self_in, const void *buf, mp_uint_t len) {
    struct _mp_obj_base_t *o = (struct _mp_obj_base_t *)self_in;
    if (o->type->stream_p == NULL || o->type->stream_p->write == NULL) {
        // CPython: io.UnsupportedOperation, OSError subclass
        nlr_raise(mp_obj_new_exception_msg(&mp_type_OSError, "Operation not supported"));
    }

    int error;
    mp_uint_t out_sz = o->type->stream_p->write(self_in, buf, len, &error);
    if (out_sz == MP_STREAM_ERROR) {
        if (mp_is_nonblocking_error(error)) {
            // http://docs.python.org/3/library/io.html#io.RawIOBase.write
            // "None is returned if the raw stream is set not to block and
            // no single byte could be readily written to it."
            // This is for consistency with read() behavior, still weird,
            // see abobe.
            return mp_const_none;
        }
        nlr_raise(mp_obj_new_exception_arg1(&mp_type_OSError, MP_OBJ_NEW_SMALL_INT(error)));
    } else {
        return MP_OBJ_NEW_SMALL_INT(out_sz);
    }
}
As far as I can tell, mp_stream_write() is only used directly in py/modbuiltins.c and py/modsys.c; all other code that wants to write to a stream links to mp_stream_write_obj. There is no equivalent mp_stream_read() function, only functions linking to mp_stream_read_obj (or mp_stream_readall_obj or mp_stream_readinto_obj)

Would it be a problem to introduce a public mp_stream_read() function, or should I be using mp_stream_read_obj instead? If the latter, any tips about how to implement that?

Thanks!
Bryan

pfalcon
Posts: 1155
Joined: Fri Feb 28, 2014 2:05 pm

Re: Stream read and write methods

Post by pfalcon » Mon Nov 02, 2015 7:22 pm

You definitely should not be using mp_stream_read_obj for anything, but to bind it to a Python symbol. If mp_stream_read() convenience function would be helpful to you, we can introduce it.
Awesome MicroPython list
Pycopy - A better MicroPython https://github.com/pfalcon/micropython
MicroPython standard library for all ports and forks - https://github.com/pfalcon/micropython-lib
More up to date docs - http://pycopy.readthedocs.io/

blmorris
Posts: 348
Joined: Fri May 02, 2014 3:43 pm
Location: Massachusetts, USA

Re: Stream read and write methods

Post by blmorris » Mon Nov 02, 2015 7:49 pm

Okay, thanks. I take it that the reason we don't have it yet is that it hasn't been needed.

As I understand it, he have 'mp_stream_write()' as the publicly available function, which is wrapped inside of 'stream_write_method()', and this is what gets linked to mp_stream_write_obj.

Creating a parallel 'mp_stream_read()' would imply factoring the core function out of the currently existing 'stream_read()', which is the function that currently gets linked to mp_stream_read_obj. How this might be implemented isn't immediately obvious to me, however, as stream_read is more complicated than mp_stream_write given that it needs to be Unicode-aware.

It might break fewer things to do the reverse, and provide a public 'mp_stream_read()' function that is a wrapper for 'stream_read()', so maybe I'll see if I can make that work first.

-Bryan

blmorris
Posts: 348
Joined: Fri May 02, 2014 3:43 pm
Location: Massachusetts, USA

Re: Stream read and write methods

Post by blmorris » Mon Nov 02, 2015 10:35 pm

I'm having second thoughts about creating a public mp_stream_read function from the existing stream_read. What I need is to copy data from an open stream or file object into a pre-allocated array in C. stream_read is written to return a python string object, if I use that then I would need to go through the additional step of copying data from the string object into my array. This extra step removes any benefit from trying to reuse the existing function.

I think my best bet is to provide my own compact function for reading data from a stream into a buffer; it will do the same error handling that stream_read does, but since it will be purely data-oriented it won't need to deal with Unicode checking or creating a Python string.
If it could be used elsewhere, it can be factored out of the I2S code when the need arises. With that in mind, I will plan to make it compatible with mp_buffer_info_t objects.

-Bryan

pfalcon
Posts: 1155
Joined: Fri Feb 28, 2014 2:05 pm

Re: Stream read and write methods

Post by pfalcon » Mon Nov 02, 2015 10:54 pm

From my point of view, the case is clear, and such convenience function is needed. The only thing I have reservations for is whether I'd like to use it in streams.c itself. I remember when @dpgeorge asked: So, is that how feature X is going to work: [sequence of 5-6 levels deep function calls]? The only answer I could have is "yes". If factor out type->read() call to yet another function, it will be 6-7 levels deep calls. Then I'm not sure it's good for stack usage (even though it'll definitely cut the code size).

And if streams.c won't use it, then there will be no users of it in the codebase, and that's the answer why it doesn't exist. Once again, it could if you're sure it'll help you.
Awesome MicroPython list
Pycopy - A better MicroPython https://github.com/pfalcon/micropython
MicroPython standard library for all ports and forks - https://github.com/pfalcon/micropython-lib
More up to date docs - http://pycopy.readthedocs.io/

Post Reply