extmod/modlwip: slip: Use stream protocol and be port-independent. #1528
Conversation
Based on the original patch by Galen Hazelwood micropython/micropython#1517
|
Ok, this is updated patch as posted previously to #1517. Changes include: slip's stream object is now properly tracked via root pointers; builds out of the box for stmhal (mostly due to previous changes.) So, MP_MODLWIP_ROOT_POINTERS is the original idea how to try to abstract py/* from knowing about extmod/modlwip, but it seems to be a moot point after all, as py/objmodule.c & py/builtin.h already knows about it, so I'd rather rework it for py/mpstate.h to pull it automagically and not burden each and every port. |
|
Complete instruction for basic testing with unix port: lwIP doesn't appear to be 64-bit clean, so build as: under root, start slattach built from https://github.com/Granjow/net-tools and note the pts name it gives: Make pts accessible to normal user used to run uPy: Configure the slip interface: Run slip startup script: slip.py: In another terminal, ping uPy: |
|
Still not tested/debugged on stmhal, that's next step. |
|
Ok, testing with pyb.USB_VCP() and busy-wait handling - the board locks up after receiving 2 pings. @galenhz : You did ping tests with your previous code, don't you? |
|
Ok, lock up is not lock up, it's just when slattach starts, it grabs serial line, so picocom is deaf. Killing slattach revives it, and seems that code simply throws Python-level exception after 2nd ping, so everything ends up in REPL. |
|
LOL. It just received byte value which it interpreted as Ctrl+C. USB_VCP.setinterrupt(-1) fixes that. 250 pings and counting. So, it's already fair to say that this patch works as expected on stmhal, though I hope to solder a corner UART pins and test UART too. |
|
Following works as expected: One thing I noticed is that calling lwip.reset() 2nd time makes lwip non-responsive. So, makes sense to name it .init() after all (and makes sense to call automatically). |
|
With timer callback: I cannot reproduce REPL hanging, it works for me all the time. However, timeout=0 doesn't work as expected for UART object, current workaround is to use timeout=1. And well, default buffer of UART object is not enough to handle pings, as @galenhz's original example in #1517 shows. Following works for me as expected (with working REPL): |
|
As a full disclosure, I did most of the tests above with MTU set conservatively to 128 on SLIP interface on Linux side, though that's unlikely to matter for ping tests. |
Totally agree to rework it. We could define a MICROPY_EXTMOD_ROOT_POINTERS macro, and have it contain all relevant pointers, then put this into mpstate.h. But really, the simplest thing is to just add, one-by-one, the pointers directly into mpstate.h, ie in this case: diff --git a/py/mpstate.h b/py/mpstate.h
index dd185a7..1502895 100644
--- a/py/mpstate.h
+++ b/py/mpstate.h
@@ -127,6 +127,10 @@ typedef struct _mp_state_vm_t {
// include any root pointers defined by a port
MICROPY_PORT_ROOT_POINTERS
+ // root pointers for extmod
+
8000
#if MICROPY_PY_LWIP_SLIP
+ mp_obj_t lwip_slip_stream;
+ #endif
+
//
// END ROOT POINTER SECTION
////////////////////////////////////////////////////////////
|
But maybe there's a reason for reset? Can we make .reset() actually do a proper reset? Otherwise, indeed just call it automatically (and don't even provide a .init() method). |
|
I can confirm that unix build works as described above. For those using "ip" instead of "ifconfig" the command is: When I press ctrl-C to kill the uPy process I get a seg fault. @pfalcon does that happen to you? |
Yep, also in #1035, micropython/micropython-lib#24 . So, there's in general something wrong with x86 port (something which of course doesn't happen under debugger, so really need to dig core files or something). |
Yes, that's what I meant, I'll rework the patch like that. |
|
I can confirm that stmhal build (on pyboard) works as described above, using both "while 1" loop and Timer(7) callback (at 20hz and 100hz). I'm happy to merge once the root pointer stuff is reworked. Awesome! |
|
Updated and merged. |
Based on the original patch by Galen Hazelwood
#1517