Skip to content
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.

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

Update Nina-W10 driver to support firmware 1.5.0 #8654

Closed
wants to merge 6 commits into from

Conversation

iabdalkader
Copy link
Contributor

@iabdalkader iabdalkader commented May 11, 2022

Depends on the following PR(s):

Fix the following issue(s):

Testing:

  • Unittests results in the second comment.
  • Tested webrepl (foreground and background modes).
  • Tested microdot (sync and aysnc modes).
  • Tested uping, ufptd and utelnetserver.

@iabdalkader
Copy link
Contributor Author

Standard tests results:

./run-multitests.py -i pyb:a0 multi_net/*

multi_net/ssl_data.py on ttyACM0|micropython: pass
multi_net/ssl_data.py.exp on : pass
multi_net/tcp_accept_recv.py on ttyACM0|micropython: pass
multi_net/tcp_client_rst.py on ttyACM0|micropython: FAIL
multi_net/tcp_data.py on ttyACM0|micropython: pass
multi_net/uasyncio_tcp_client_rst.py on ttyACM0|micropython: pass
multi_net/uasyncio_tcp_client_rst.py.exp on : pass
multi_net/uasyncio_tcp_close_write.py on ttyACM0|micropython: pass
multi_net/uasyncio_tcp_close_write.py.exp on : pass
multi_net/uasyncio_tcp_readexactly.py on ttyACM0|micropython: pass
multi_net/uasyncio_tcp_readexactly.py.exp on : pass
multi_net/uasyncio_tcp_readinto.py on ttyACM0|micropython: pass
multi_net/uasyncio_tcp_readinto.py.exp on : pass
multi_net/uasyncio_tcp_server_client.py on ttyACM0|micropython: pass
multi_net/uasyncio_tcp_server_client.py.exp on : pass
multi_net/udp_data.py on ttyACM0|micropython: pass
16 tests performed
15 tests passed
1 tests failed: multi_net/tcp_client_rst.py
./run-multitests.py -i micropython -i pyb:a0 multi_net/*

multi_net/ssl_data.py on micropython|ttyACM0: pass
multi_net/ssl_data.py.exp on : pass
multi_net/tcp_accept_recv.py on micropython|ttyACM0: pass
multi_net/tcp_client_rst.py on micropython|ttyACM0: skip
multi_net/tcp_data.py on micropython|ttyACM0: pass
multi_net/uasyncio_tcp_client_rst.py on micropython|ttyACM0: skip
multi_net/uasyncio_tcp_client_rst.py.exp on : pass
multi_net/uasyncio_tcp_close_write.py on micropython|ttyACM0: FAIL
multi_net/uasyncio_tcp_close_write.py.exp on : pass
multi_net/uasyncio_tcp_readexactly.py on micropython|ttyACM0: pass
multi_net/uasyncio_tcp_readexactly.py.exp on : pass
multi_net/uasyncio_tcp_readinto.py on micropython|ttyACM0: pass
multi_net/uasyncio_tcp_readinto.py.exp on : pass
multi_net/uasyncio_tcp_server_client.py on micropython|ttyACM0: pass
multi_net/uasyncio_tcp_server_client.py.exp on : pass
multi_net/udp_data.py on micropython|ttyACM0: FAIL
16 tests performed
12 tests passed
2 tests skipped: multi_net/tcp_client_rst.py multi_net/uasyncio_tcp_client_rst.py
2 tests failed: multi_net/uasyncio_tcp_close_write.py multi_net/udp_data.py
python run-tests.py --target pyboard net_inet/*.py net_hosted/*.py

pass  net_inet/getaddrinfo.py
pass  net_inet/ssl_errors.py
FAIL  net_inet/test_tls_nonblock.py
FAIL  net_inet/test_tls_sites.py
pass  net_inet/tls_num_errors.py
pass  net_inet/tls_text_errors.py
pass  net_inet/uasyncio_cancel_stream.py
pass  net_inet/uasyncio_open_connection.py
pass  net_inet/uasyncio_tcp_read_headers.py
pass  net_hosted/accept_nonblock.py
pass  net_hosted/accept_timeout.py
pass  net_hosted/connect_nonblock.py
FAIL  net_hosted/connect_nonblock_xfer.py
pass  net_hosted/connect_poll.py
pass  net_hosted/ssl_getpeercert.py
pass  net_hosted/uasyncio_start_server.py
16 tests performed (130 individual testcases)
13 tests passed
3 tests failed: connect_nonblock_xfer test_tls_nonblock test_tls_sites
  • Note the following tests fail on PYB_SF2:
multi_net/tcp_client_rst.py
net_hosted/connect_nonblock_xfer
net_inet/test_tls_nonblock
net_inet/test_tls_sites.py

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2022

Codecov Report

Merging #8654 (eaaf6e9) into master (e3c880a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8654   +/-   ##
=======================================
  Coverage   98.22%   98.22%           
=======================================
  Files         154      154           
  Lines       20331    20331           
=======================================
  Hits        19970    19970           
  Misses        361      361           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3c880a...eaaf6e9. Read the comment docs.

@robert-hh
Copy link
Contributor

@iabdalkader I did not find information about the driver 1.5.0. Was was changed since 1.4.8, and where is the link?

@iabdalkader
Copy link
Contributor Author

@iabdalkader I did not find information about the driver 1.5.0. Was was changed since 1.4.8, and where is the link?

It's still a work in progress, not committed or released yet (because it may change). It's basically the same firmware but adds a proper BSD-like sockets API. let me know if you want to test it.

@robert-hh
Copy link
Contributor

Sure. I's like to test that. Your effort is very in interesting for other ports, which have already LWIP in use but no WiFi. I had a short look into the NINA code about whether it is possible (or easy possible) to implement a raw packet transport interface. But did not follow that further.
b.t.w: It is a pity that there is no small breakout for the cyw43 module.

@iabdalkader
Copy link
Contributor Author

iabdalkader commented May 12, 2022

  • If you want to build the firmware from source follow the guide and all the changes are in this commit. If not, I attached the binaries.

  • Next upload the SerialNinaPassthrough Arduino sketch from Arduino IDE and run the following command:

python ~/esp-idf/components/esptool_py/esptool/esptool.py --chip esp32 --port /dev/ttyACM0 --baud 115200 --before no_reset --after hard_reset write_flash -z --flash_mode dio --flash_freq 40m --flash_size detect 0x1000 bootloader.bin 0xf000 phy_init_data.bin 0x30000 nina-fw.bin 0x8000 partitions.bin

firmware:
ninafw-1.5.0.zip

@robert-hh
Copy link
Contributor

Thanks. I'll try to make the set-up tomorrow.

@robert-hh
Copy link
Contributor

The upload of the NINA firmware failed. Do I have to change the passthru script. I noticed that the default version sets a baud rate of 9600, which looks wrong. For the PC side, 115200 seems appropriate, but what about the ESP32 side?
Is it Serial1, at which speed?

@iabdalkader
Copy link
Contributor Author

Try this command, note the no_reset flag

python ~/esp-idf/components/esptool_py/esptool/esptool.py --chip esp32 --port /dev/ttyACM0 --baud 115200 --before no_reset --after hard_reset write_flash -z --flash_mode dio --flash_freq 40m --flash_size detect 0x1000 bootloader.bin 0xf000 phy_init_data.bin 0x30000 nina-fw.bin 0x8000 partitions.bin

@robert-hh
Copy link
Contributor

OK. There was a problem with creating SerialNinaPassthrough, but now I could upload the NINA fw.

@iabdalkader
Copy link
Contributor Author

Great, I force pushed some more fixes here to improve timeout handling.

@robert-hh
Copy link
Contributor

So I have compiled the firmware, uploaded it and got a version v1.18-425. On boot it tells that the NINA FW is 1.5.0.
I can connect to my AP, and can Ping the device. But that's it. A simple data send script fails at the connect attempt. 10.0.0.152 is the IP4 address of my PC, where I just run the command:
nc -l 1234 | wc -c
The same script runs fine on an ESP32.

import socket
from time import sleep_us, ticks_ms
blen = 1024
b = bytearray(blen)

def xmit(count, delay=1000):
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    s.connect(("10.0.0.152", 1234))
    mod = count // 100
    if mod < 10:
        mod = 10
    if mod > 10000:
        mod = 10000
    start = ticks_ms()
    sent = 0
    for i in range(count):
        try:
            s.sendall(b)
            sent += blen
            sleep_us(delay)
            if (i % mod) == 0:
                print(i)
        except Exception as e:
            print(i)
            print(e)
            break
    stop = ticks_ms()
    s.close()
    print(sent, "bytes sent", (stop - start)/count, "ms/frame")

@iabdalkader
Copy link
Contributor Author

A simple data send script fails at the connect attempt

That's not likely because most of the tests pass. Try running the unit tests above, if all is good with the firmware and driver, you should see the exact same results.

@iabdalkader
Copy link
Contributor Author

I have compiled the firmware

If you build the firmware from source, this patch in esp-idf might be necessary:

git diff components/lwip/port/esp32/include/arch/cc.h
diff --git a/components/lwip/port/esp32/include/arch/cc.h b/components/lwip/port/esp32/include/arch/cc.h
index cba0b365ea..74bf26c884 100644
--- a/components/lwip/port/esp32/include/arch/cc.h
+++ b/components/lwip/port/esp32/include/arch/cc.h
@@ -35,7 +35,9 @@
 #define __ARCH_CC_H__
 
 #include <stdint.h>
+#ifndef LWIP_PROVIDE_ERRNO
 #include <errno.h>
+#endif
 #include <assert.h>
 #include <stdio.h>
``

@robert-hh
Copy link
Contributor

I just can show you what's happening. If fails at socket connect, in that it immediately closes the socket again. See the Wireshark log below. I used the NINA firmware which you have sent. The RP2040 firmware is based on your PR8654 state. Maybe you are using a different RP2040 firmware.

rp2040_connect

In contrast the packet sequence for a socket connect with an ESP32:

esp32_connect

@iabdalkader
Copy link
Contributor Author

iabdalkader commented May 14, 2022

Maybe you are using a different RP2040 firmware

I'm using the changes in this PR with all of the fixes to modusocket which I mentioned and SSL enabled. Does the unit tests work ?

btw in extmod/network_ninaw10.c you can uncomment the following to see a trace of all the called functions:

#define debug_printf(...)   // mp_printf(&mp_plat_print, __VA_ARGS__)

@robert-hh
Copy link
Contributor

Most of the unittests fail. So most likely the RP2040 binaries are different. Do you build your binary from your nina_fw_1.5.0 branch?

@iabdalkader
Copy link
Contributor Author

iabdalkader commented May 14, 2022

Most of the unittests fail. So most likely the RP2040 binaries are different. Do you build your binary from your nina_fw_1.5.0 branch?

I build the firmware in branch that cherry-picks the nina_fw_1.5.0, and all of the fixes in #8658 and SSL support in #8651

Are you connected on boot ? I have this in boot.py

import network
SSID=''
KEY=''
wlan = network.WLAN(network.STA_IF)
wlan.active(True)
wlan.connect(SSID, KEY)

@robert-hh
Copy link
Contributor

OK. I did not add the modusocket changes (yet).
And yes: I connect on boot.

@robert-hh
Copy link
Contributor

Adding the commits from #8658 changed the picture. Now my little script only complains about missing the method sendall(). But using send instead works. Speed is about 60 kb/s. A simple ftp server works. The uftpd misses the option 20 for SOL_SOCKET, where a handler can be registered for socket connect events. That one is used for webrepl.
For the standard test suites cited above, you have to explain the set-up. Just using a single board, the Pass/Fail figures I get are different.

@iabdalkader
Copy link
Contributor Author

The uftpd misses the option 20 for SOL_SOCKET, where a handler can be registered for socket connect events. That one is used for webrepl.

Yeah I know, I mentioned that in my first comment. I'm working on something might be able to support it for webrepl.

For the standard test suites cited above, you have to explain the set-up. Just using a single board, the Pass/Fail figures I get are different.

I run tests against micropython in unix port.

@iabdalkader
Copy link
Contributor Author

I have a fix for the missing socket callback (sockopt 20) which fixes both webrepl (in background mode) and microdot in sync mode.

@robert-hh
Copy link
Contributor

I run tests against micropython in unix port.

So how do you set the test up? Do you simply run the test scripts from the command line?

@robert-hh
Copy link
Contributor

I have a fix for the missing socket callback (sockopt 20) which fixes both webrepl (in background mode) and microdot in sync mode.

Sounds good.

@iabdalkader
Copy link
Contributor Author

So how do you set the test up? Do you simply run the test scripts from the command line?

Build MicroPython in unix port, then connect the board and wait for it to connect to API and then run the commands in my second comment on this PR above.

@robert-hh
Copy link
Contributor

then run the commands in my second comment on this PR above.

These commands work with CPython, but not with micropython.

@robert-hh
Copy link
Contributor

I did not add the SSL PR yet. With that, the result from the first test set is identical, for the second test I get:

./run-multitests.py -i micropython -i pyb:a0 multi_net/*

16 tests performed
6 tests passed
2 tests skipped: multi_net/tcp_client_rst.py multi_net/uasyncio_tcp_client_rst.py
8 tests failed: multi_net/ssl_data.py multi_net/tcp_accept_recv.py multi_net/tcp_data.py multi_net/uasyncio_tcp_close_write.py multi_net/uasyncio_tcp_readexactly.py multi_net/uasyncio_tcp_readinto.py multi_net/uasyncio_tcp_server_client.py multi_net/udp_data.py

For the last test I get a full fail:

python run-tests.py --target pyboard net_inet/*.py net_hosted/*.py
16 tests performed (117 individual testcases)
0 tests passed
16 tests failed: accept_nonblock accept_timeout connect_nonblock connect_nonblock_xfer connect_poll getaddrinfo ssl_errors ssl_getpeercert test_tls_nonblock test_tls_sites tls_num_errors tls_text_errors uasyncio_cancel_stream uasyncio_open_connection uasyncio_start_server uasyncio_tcp_read_headers

@iabdalkader
Copy link
Contributor Author

iabdalkader commented May 14, 2022

@robert-hh I don't think you have all of the required changes for some reason, maybe something is out of sync, so it might be a good idea to just start over from scratch by pulling this PR (there should be 3 commits now), along with all the commits in modusocket fixes (which now include a needed fix for sockopt 20) and the SSL support PR (a lot of tests depend on SSL), and webrepl PR if you want to test it. Alternatively you can just try the attached firmware which I just built with everything enabled.

If it helps, here's the list of commits I have cherry-picked in a local testing branch:

pick da5c261e6 rp2: Enable SSL module with mbedtls.
pick 02ddf017f drivers/ninaw10: Update driver to support firmware 1.5.0.
pick 842cf120e extmod/network_ninaw10.c: Add support for socket events callback.
pick f021b78fa drivers/ninaw10: Disable scheduling when sending SPI commands.
pick 0957b9114 extmod/modusocket.c: Fix new socket poll flags.
pick 87bfd3171 extmod/modusocket.c: Bind socket to default NIC in setsockopt.
pick 952c19598 extmod/modusocket.c: Fix errcode returned from socket read/write.
pick 1fbdc3183 extmod/modusocket.c: Add socket type print function.
pick 518c1a159 extmod/modusocket.c: Add support for socket events callback.
pick c51bfa595 rp2/boards/ARDUINO_NANO_RP2040_CONNECT: Enable webrepl.

Note for your second test run, it looks like the board is Not connected. I usually have a main.py blinky that I wait for before running the tests to make sure boot.py is done and the board is connected (you could turn on an LED in boot.py after connect).

With the firmware attached, I tested webrepl, microdot and re-ran all the unitests again, and everything seems to be working perfectly (except for the few tests that always fail of course).

firmware.uf2.zip

@robert-hh
Copy link
Contributor

Looks fine now. I had the network connect in main.py, not boot.py Moving it to boot.py changed the picture.

/run-multitests.py -i micropython -i pyb:a0 multi_net/*

16 tests performed
12 tests passed
2 tests skipped: multi_net/tcp_client_rst.py multi_net/uasyncio_tcp_client_rst.py
2 tests failed: multi_net/uasyncio_tcp_close_write.py multi_net/udp_data.py
python run-tests.py --target pyboard net_inet/*.py net_hosted/*.py

16 tests performed (117 individual testcases)
12 tests passed
4 tests failed: connect_nonblock_xfer test_tls_nonblock test_tls_sites tls_text_errors

@iabdalkader
Copy link
Contributor Author

Looks fine now. I had the network connect in main.py, not boot.py Moving it to boot.py changed the picture.

I think raw repl doesn't run main.py only runs boot.py.. Anyway, that's great! What about webrepl, telnet, ftp and all that ? If you can confirm my results then I think it's ready for release.

@robert-hh
Copy link
Contributor

What about webrepl, telnet, ftp and all that ?

These work fine. The problem with the pye editor also disappeared magically.

@iabdalkader
Copy link
Contributor Author

The problem with the pye editor also disappeared magically

It was the same issue with sockets poll list getting collected.

@iabdalkader iabdalkader force-pushed the nina_fw_1.5.0 branch 3 times, most recently from ecf64aa to c818d76 Compare May 17, 2022 10:40
@iabdalkader
Copy link
Contributor Author

iabdalkader commented May 17, 2022

FYI I switched to system error numbers in nina-fw due to an issue with some of the esp-idf components which assume the standard errno is used (not LWIP's errno). The end result is about the same (except for 1 test). The test results haven't changed, however, when changes are merged, and the nina firmware is merged, it will have to be updated again to match the driver's error numbers.

@iabdalkader
Copy link
Contributor Author

Rebased on latest changes.

extmod/network_ninaw10.c Outdated Show resolved Hide resolved
extmod/network_ninaw10.c Outdated Show resolved Hide resolved
extmod/network_ninaw10.c Outdated Show resolved Hide resolved
extmod/network_ninaw10.c Outdated Show resolved Hide resolved
@iabdalkader iabdalkader force-pushed the nina_fw_1.5.0 branch 2 times, most recently from 7aaa377 to f6346cd Compare May 25, 2022 13:39
@dpgeorge
Copy link
Member

Merged in d037e75 through 0bdfcea

@iabdalkader
Copy link
Contributor Author

Note the linked issues should be fixed, after updating the firmware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants