ports/unix: Add full uos.dupterm support. #6080

andrewleech · 2020-05-27T06:40:21Z

The unix port currently supports a very limited uos.dupterm implementation.
It can only replace stdin and/or duplicate stdout.
When using uos.dupterm to replace slot 0 it does not return a handle to the existing (posix stdio) stream.

This PR resolves these issues, preferring to use dupterm for all stdio on unix when MICROPY_PY_OS_DUPTERM=1 is configured in the build.

With this change simple modification of the output stream is possible, for example to format logging messaging with a basic timestamp

stdio = None

class logger:
    def write(self, buf):
        global stdio        
        stdio.write(str(utime.time()))
        stdio.write(" | ")
        stdio.write(buf)

stdio = uos.dupterm(logger(), 0)

dlech · 2020-05-27T14:34:32Z

ports/unix/mphalport.h

@@ -103,3 +103,7 @@ enum {

 void mp_hal_get_mac(int idx, uint8_t buf[6]);
 #endif
+
+#if MICROPY_PY_OS_DUPTERM
+void init_dupterm_stdio();


This should have mp_ prefix since it is in a header file. Or even better, mp_unix_ since it is specific to the unix port.

I was loath to change the name of an existing file, but I too thought it was oddly inconsistent in name.

dlech · 2020-05-27T14:41:10Z

ports/unix/unix_mphal.c

@@ -64,8 +66,11 @@ STATIC void sighandler(int signum) {
 }
 #endif

+int mp_interrupt_char = -1;


might be nice to have a comment that explains this is the mp_interrupt_char from lib/utils/interupt_char.h

Good point. Yes I had to add this just to allow compilation with MICROPY_PY_OS_DUPTERM enabled.

dlech · 2020-05-27T14:53:35Z

ports/unix/unix_mphal.c

-        MP_STATE_VM(dupterm_objs[idx]) = save_term;
+STATIC mp_uint_t unix_stdio_read(mp_obj_t self_in, void *buf, mp_uint_t size, int *errcode) {
+    ssize_t ret;
+    MP_HAL_RETRY_SYSCALL(ret, read(STDIN_FILENO, (byte *)buf, size), {});


should this be handling the error? Otherwise we end up returning -1 at the end of the function without setting *errorcode

dlech · 2020-05-27T14:54:12Z

ports/unix/unix_mphal.c

+STATIC mp_uint_t unix_stdio_read(mp_obj_t self_in, void *buf, mp_uint_t size, int *errcode) {
+    ssize_t ret;
+    MP_HAL_RETRY_SYSCALL(ret, read(STDIN_FILENO, (byte *)buf, size), {});
+    if (ret == 0) {


I thought a return value of 0 meant that we have reached EOF rather than non-blocking. Is this a special case for if stdin is closed but other slots are still open?

dlech · 2020-05-27T15:07:07Z

ports/unix/unix_mphal.c

+
+STATIC mp_uint_t unix_stdio_write(mp_obj_t self_in, const void *buf, mp_uint_t size, int *errcode) {
+    int ret;
+    MP_HAL_RETRY_SYSCALL(ret, write(STDOUT_FILENO, (const byte *)buf, size), {});


same here with not handling the error

dpgeorge · 2020-05-28T00:43:30Z

On CPython I guess you can hook into stdout by redefining sys.stdout... and maybe that's how MicroPython should have done it from the start, instead of adding uos.dupterm() (although it'd still need an efficient multiplexer, eg sys.stdout = micropython.stream_multiplex(sys.stdout, machine.UART(...)), not to mention handling sys.stdin).

Anyway, the way it is now sys.stdout is before the dupterm multiplexer, so writing to sys.stdout goes to all dupterm streams (similar for reading). And what this PR does is adds raw-stdout/stdin objects, ie they do not go via dupterm. On bare-metal the equivalent would be a UART or USB_VCP object.

It just seems like there's a lot of complexity here with two different kinds of stdout/stdin objects (a dupterm one and a raw one). But I don't really see a way around that.

Would it be possible to reuse the extmod/vfs_posix_file.c code to implement the raw stdin/stdout? That code has compile-time dupterm support but maybe that dupterm support can be made optional at runtime (via a flag in the stdio object) so that the same code can be used for both sets of stdio objects?

andrewleech · 2020-05-28T04:07:50Z

Thanks for the pointer towards extmod/vfs_posix_file.c - it already has the stdio object instantiated

const mp_obj_vfs_posix_file_t mp_sys_stdin_obj = {{&mp_type_textio}, STDIN_FILENO};
const mp_obj_vfs_posix_file_t mp_sys_stdout_obj = {{&mp_type_textio}, STDOUT_FILENO};
const mp_obj_vfs_posix_file_t mp_sys_stderr_obj = {{&mp_type_textio}, STDERR_FILENO};

I should be able to clean up this patch reusing them.

andrewleech · 2020-05-28T06:34:11Z

@dpgeorge there's a catch with using the mp_sys_stdin_obj etc objects above as i/o for dupterm, other things read/write to these objects - and writes to stdout/stderr are redirected to dupterm (hello recursion).

I've resolved this by adding a relatively small amount of extra logic to make a new const mp_obj_vfs_posix_file_t mp_sys_raw_stdio_obj which will automatically read/write to the correct fd and not go via dupterm - this new raw_stdio can be used for unix_mphal.c both with and without dupterm.

It feels like a bit of a tacked-on-the-side addition, but it does make for less duplication than I had previously.

Now I'm also getting a lot of travis fails, for one the minimal port doesn't have the new file handle created, presumably due to a lack of one/both of MICROPY_VFS_POSIX / MICROPY_VFS_POSIX_FILE
Some of the others are failing a number of unit tests, don't know why yet.

andrewleech force-pushed the unix_dupterm branch from 42bfad9 to 7199c61 Compare May 27, 2020 10:01

dlech reviewed May 27, 2020

View reviewed changes

pi-anl added 2 commits November 14, 2020 16:10

ports/unix: Add full uos.dupterm support.

dd803e8

ports/unix: Add full uos.dupterm support.

da3f655

andrewleech force-pushed the unix_dupterm branch from 372e90b to da3f655 Compare November 15, 2020 23:48

dpgeorge added the port-unix label Nov 30, 2021

This was referenced Feb 29, 2024

global: Remove the STATIC macro. #13763

Merged

global: Prune trailing whitespace. #13777

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ports/unix: Add full uos.dupterm support. #6080

ports/unix: Add full uos.dupterm support. #6080

andrewleech commented May 27, 2020

dlech May 27, 2020

andrewleech May 28, 2020

dlech May 27, 2020

andrewleech May 28, 2020

dlech May 27, 2020

dlech May 27, 2020

dlech May 27, 2020

dpgeorge commented May 28, 2020

andrewleech commented May 28, 2020

andrewleech commented May 28, 2020 •

edited

Loading

ports/unix: Add full uos.dupterm support. #6080

Are you sure you want to change the base?

ports/unix: Add full uos.dupterm support. #6080

Conversation

andrewleech commented May 27, 2020

dlech May 27, 2020

Choose a reason for hiding this comment

andrewleech May 28, 2020

Choose a reason for hiding this comment

dlech May 27, 2020

Choose a reason for hiding this comment

andrewleech May 28, 2020

Choose a reason for hiding this comment

dlech May 27, 2020

Choose a reason for hiding this comment

dlech May 27, 2020

Choose a reason for hiding this comment

dlech May 27, 2020

Choose a reason for hiding this comment

dpgeorge commented May 28, 2020

andrewleech commented May 28, 2020

andrewleech commented May 28, 2020 • edited Loading

andrewleech commented May 28, 2020 •

edited

Loading