8000 extmod/modlwip: slip: Use stream protocol and be port-independent. by pfalcon · Pull Request #1528 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

extmod/modlwip: slip: Use stream protocol and be port-independent. #1528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

extmod/modlwip: slip: Use stream protocol and be port-independent. #1528

wants to merge 1 commit into from

Conversation

pfalcon
Copy link
Contributor
@pfalcon pfalcon commented Oct 24, 2015

Based on the original patch by Galen Hazelwood
#1517

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 24, 2015

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.

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 24, 2015

Complete instruction for basic testing with unix port:

lwIP doesn't appear to be 64-bit clean, so build as:

make MICROPY_PY_LWIP=1 MICROPY_PY_LWIP_SLIP=1 MICROPY_FORCE_32BIT=1

under root, start slattach built from https://github.com/Granjow/net-tools and note the pts name it gives:

sudo ./slattach -L -p slip -v /dev/ptmx
slip started on /dev/ptmx ptsname /dev/pts/45 interface sl0

Make pts accessible to normal user used to run uPy:

sudo chown $(whoami) /dev/pts/45

Configure the slip interface:

sudo ifconfig sl0 192.168.5.1 pointopoint 192.168.5.2 mtu 1500

Run slip startup script:

micropython ./slip.py /dev/pts/45

slip.py:

import sys
import os
import lwip
import utime

# Run as: micropython slip.py /dev/pts/115

fd = os.open(sys.argv[1], os.O_RDWR | os.O_NONBLOCK)
f = open(fd, "rb")

lwip.reset()
sl = lwip.slip(f, "192.168.5.2", "192.168.5.1")
while 1:
    lwip.callback()
#    print(".", end="")
    utime.sleep(1 / 20)

In another terminal, ping uPy:

ping 192.168.5.2
PING 192.168.5.2 (192.168.5.2) 56(84) bytes of data.
64 bytes from 192.168.5.2: icmp_seq=1 ttl=255 time=30.2 ms
64 bytes from 192.168.5.2: icmp_seq=2 ttl=255 time=31.2 ms

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 24, 2015

Still not tested/debugged on stmhal, that's next step.
8000

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 24, 2015

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?

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 24, 2015

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.

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 25, 2015

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.

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 25, 2015

Following works as expected:

import pyb
import lwip
import utime

u = pyb.UART(4, 115200)

lwip.reset()
sl = lwip.slip(u, "192.168.5.2", "192.168.5.1")
while 1:
    lwip.callback()
    #print(".", end="")
    utime.sleep(1 / 5)

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).

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 25, 2015

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):

import pyb
import lwip
import utime

u = pyb.UART(4, 115200, read_buf_len=3100, timeout=1)

lwip.reset()
sl = lwip.slip(u, "192.168.5.2", "192.168.5.1")
def cb(t):
    lwip.callback()
#    print(".", end="")
tim = pyb.Timer(7)
tim.init(freq=20)
tim.callback(cb)

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 25, 2015

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.

@dpgeorge
Copy link
Member

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.

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 
+    #if MICROPY_PY_LWIP_SLIP
+    mp_obj_t lwip_slip_stream;
+    #endif
+
     //
     // END ROOT POINTER SECTION
     ////////////////////////////////////////////////////////////

@dpgeorge
Copy link
Member

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).

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).

@dpgeorge
Copy link
Member

I can confirm that unix build works as described above. For those using "ip" instead of "ifconfig" the command is:

ip addr add 192.168.5.1 peer 192.168.5.2 dev sl0
ip link set sl0 up

When I press ctrl-C to kill the uPy process I get a seg fault. @pfalcon does that happen to you?

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 25, 2015

@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).

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 25, 2015

But really, the simplest thing is to just add,

Yes, that's what I meant, I'll rework the patch like that.

@dpgeorge
Copy link
Member

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!

@pfalcon
Copy link
Contributor Author
pfalcon commented Oct 26, 2015

Updated and merged.

@pfalcon pfalcon closed this Oct 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pfalcon @dpgeorge 31D4
0