8000 RP2: Add support for Arduino Nano RP2040 by iabdalkader · Pull Request #7669 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content
/ micropython Public

RP2: Add support for Arduino Nano RP2040 #7669

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 2 commits into from
Closed

RP2: Add support for Arduino Nano RP2040 #7669

wants to merge 2 commits into from

Conversation

iabdalkader
Copy link
Contributor
@iabdalkader iabdalkader commented Aug 15, 2021 •
edited
Loading

Requires and depends on the following for (WiFi/BT, MSC support and misc fixes.):

Closes #7634

NOTE: Until this patch raspberrypi/pico-sdk#537 gets merged and makes it to a stable release, Arduino Nano RP2040 boards with the newer flash part will Not work.

@iabdalkader iabdalkader force-pushed the arduino_nano_rp2040 branch 3 times, most recently from 813628e to f24de36 Compare August 15, 2021 19:11
dpgeorge
dpgeorge reviewed Aug 16, 2021
View reviewed changes
@iabdalkader iabdalkader force-pushed the arduino_nano_rp2040 branch 2 times, most recently from fca4e52 to 4aee33b Compare August 21, 2021 16:08
@iabdalkader
Copy link
Contributor Author
iabdalkader commented Aug 21, 2021 •
edited
Loading

@dpgeorge I added audio recording/streaming with pdm mic support for this board in
4aee33b it's completely self-contained within the board dir. Should it be moved up to the port level ?

@iabdalkader iabdalkader force-pushed the arduino_nano_rp2040 branch 3 times, most recently from 670095a to 22aa55e Compare August 23, 2021 12:27
@robert-hh
Copy link
Contributor
robert-hh commented Sep 12, 2021 •
edited
Loading

I found this set of PR's a little bit confusing and hard for maintainers to deal with it. I wanted to test WiFi support only for the Nano and abandoned it given the set of PRs I found. What I had expected was:
a) a single PR based on a current master, which contains all files and setting for WiFi
b) a single PR based on the PR a) adding basic BT support
c) one or more PRs based on c) adding additional BT services,
d) one more more PRs adding more board features, like a motion detector, and independent from that
e) a PR, moving modnetwork and modusocket to extmod.

Uh oh!

There was an error while loading. Please reload this page.
@iabdalkader
Copy link
Contributor Author
iabdalkader commented Sep 12, 2021 •
edited
Loading

I wanted to test WiFi support only for the Nano

Hi, you can just test our OpenMV firmware it's based on MicroPython 1.15 but it has all of these features enabled for the Nano (and some examples)

I found this set of PR's a little bit confusing and hard for maintainers to deal with it. I wanted to test WiFi support only for the Nano and abandoned it given the set of PRs I found. What I had expected was:
a) a single PR based on a current master, which contains all files and setting for WiFi

I think multiple smaller independent PRs are much easier to process, but I also don't like to cascade PRs like that because most tend to change a lot and some might get dropped...In this case I thought about moving modnetwork/modusocket to extmod only after this PR, so I sent another PR to move these modules, which I don't know yet if it's going to be merged or not, so I'm also leaving the duplicate files in the networking PR as is until it gets decided.

@iabdalkader
Copy link
Contributor Author
iabdalkader commented Sep 12, 2021 •
edited
Loading

@robert-hh I updated the list of linked PRs, you can see that most of them were independent of each other, if they were cascaded on top of each other (or combined) they would most likely still be unmerged, and none of these smaller fixes would have made it to 1.17 release (for example the flash fix and MSC support)

@iabdalkader iabdalkader force-pushed the arduino_nano_rp2040 branch 9 times, most recently from fdeb249 to 8c1c590 Compare September 19, 2021 15:40
@manpowre
Copy link
manpowre commented Oct 24, 2021 •
edited
Loading

I tried to build this PR:
make BOARD=ARDUINO_NANO_RP2040_CONNECT

and build fails.. I tried some of the other boards and they build just fine including PICO

/Users/xx/Documents/GitHub/micropython/extmod/modbluetooth.c:47:2: error: #error l2cap channels require synchronous modbluetooth events 47 | #error l2cap channels require synchronous modbluetooth events | ^~~~~ /Users/xx/Documents/GitHub/micropython/extmod/modbluetooth.c:51:2: error: #error pairing and bonding require synchronous modbluetooth events 51 | #error pairing and bonding require synchronous modbluetooth events | ^~~~~

@iabdalkader
Copy link
Contributor Author

and build fails.. I

Hi, you must pull this PR first #7668 (and optionally #7402 for MSC support) then this one, and it should build.

@iabdalkader
Copy link
Contributor Author

now it compiled.. Ill try this out on my arduino connect board.

Great, please note some boards have different flash chips, if after copying the firmware the board doesn't work you need to apply this small patch to the pico-sdk: raspberrypi/pico-sdk#537

@manpowre
Copy link

now it compiled.. Ill try this out on my arduino connect board.

Great, please note some boards have different flash chips, if after copying the firmware the board doesn't work you need to apply this small patch to the pico-sdk: raspberrypi/pico-sdk#537

I was a little too quick earlier, I accidentally compiled pyb which worked on master. now I made my own branch, and merged the #7668 and #7669 and NIC works like a charm:
`import network
nic = network.WLAN(network.STA_IF)
nic.active(True)
nic.connect('your-ssid', 'your-password')

print("active:", nic.active())

nic.ifconfig(('192.168.2.2', '255.255.255.0', '192.168.2.1', '8.8.8.8'))
print("config:", nic.ifconfig())
`

result:
active: True config: ('192.168.2.2', '255.255.255.0', '192.168.2.1', '192.168.2.1')

@iabdalkader
Copy link
Contributor Author

@dpgeorge Hi, can we get this back on track ? I'm 3 PRs away (including this one) from fully supporting this board in upstream, and the 3 remaining PRs have been ready for too long without any review or feedback.

@dpgeorge
Copy link
Member

Hi, can we get this back on track ?

Yes!

I reviewed the main WiFi driver #7668.

As for this PR, I'd really prefer if it did not include the audio driver because it makes it much harder to review, and the discussion will be longer (eg it includes an ST driver that needs to go elsewhere). Please make this PR just about adding a board definition for the RP2040, then audio can be discussed separately.

@iabdalkader
Copy link
Contributor Author
iabdalkader commented Nov 12, 2021 •
edited
Loading

I'd really prefer if it did not include the audio driver

  • Audio driver + modules removed.
  • IMU module updated to support MLC (Machine Learning Core) but it requires storage to load models.

@iabdalkader iabdalkader force-pushed the arduino_nano_rp2040 branch 2 times, most recently from 1c6821b to 6429061 Compare November 12, 2021 17:59
dpgeorge
dpgeorge reviewed Nov 13, 2021
View reviewed changes
@dpgeorge
Copy link
Member

Can you please add a board.json file for this new board? See ports/rp2/boards/PIMORONI_TINY2040/board.json for an example to copy and adjust.

@iabdalkader
Copy link
Contributor Author

Can you please add a board.json file for this new board? See ports/rp2/boards/PIMORONI_TINY2040/board.json for an example to copy and adjust.

done.

@iabdalkader iabdalkader force-pushed the arduino_nano_rp2040 branch 3 times, most recently from f971c01 to 40ff4f7 Compare November 15, 2021 12:17
dpgeorge
dpgeorge requested changes Nov 15, 2021
View reviewed changes
@dpgeorge
Copy link
Member

Thanks for taking into account the feedback so far.

It would be great if you could also please split the single commit here into two commits (in this same PR):

  • one which adds the LSM driver (including examples)
  • one which adds the new rp2 board

(I know I always go on about separating commits, but it really does help!)

@iabdalkader
Copy link
Contributor Author

It would be great if you could also please split the single commit here into two commits

Done.

@iabdalkader
Copy link
Contributor Author

There's 1 PR left (the MSC support) it would be nice if it's merged first, note it's not enabled for the port just for the Nano. It's currently needed for the IMU MLC example and later for Audio, which I will send in a following PR after everything is merged and after some refactoring.

@dpgeorge
Copy link
Member

There's 1 PR left (the MSC support) it would be nice if it's merged first, note it's not enabled for the port just for the Nano.

But if MSC is enabled for the Nano then this board is now different to all other rp2 boards. And that may lead to confusion, and questions about why it's enabled on Nano only, and the only way to solve that would be to enable MSC on all rp2 boards. It's a bit of a slippery slope.

It's currently needed for the IMU MLC example and later for Audio

It's possible to use mpremote to copy files to/from the board. Yes there is some minor set up required to get mpremote working (make sure Python is installed, then pip install mpremote), but having mpremote installed gives a lot of other useful functionality. Once you have your workflow based on mpremote you forget all about MSC.

From the discussion in #7402 there are good points for and against MSC, but it's still not clear whether MSC should be supported on rp2. And as I say above I think it'll be confusing if some rp2 boards support it and some don't.

I would rather merge this PR now, so it's not blocked by the MSC discussion, and continue the MSC discussion in #7402.

@iabdalkader
Copy link
Contributor Author

I would rather merge this PR now, so it's not blocked by the MSC discussion

I agree on that let's revisit later.

@dpgeorge
Copy link
Member F438

Ok, merged in f082793 and c3dceb1 !

I tested it and it works well. But the results returned from WLAN scan do not have the same format as other WLAN drivers, that will need to be fixed.

@dpgeorge dpgeorge closed this Nov 16, 2021
@iabdalkader
Copy link
Contributor Author

Great thanks!

that will need to be fixed

I will check this and send a PR.

@iabdalkader
Copy link
Contributor Author

This should be closed #7634

@iabdalkader
Copy link
Contributor Author

But the results returned from WLAN scan do not have the same format as

Question, does it have to match the data types too ? because I return a nicely formatted mac address (vs just bytes).

@dpgeorge
Copy link
Member

does it have to match the data types too

Yes. Otherwise code will not be compatible across devices. Eg I have a script that does wifi scanning and prints out a nice result where the MAC is formatted, and if the MAC is already formatted then it will get double formatted.

@iabdalkader
Copy link
Contributor Author

Yes. Otherwise code will not be compatible across devices.

It's now the same order/data type as other NICs, plus small fix to netinfo command #8004

@iabdalkader iabdalkader deleted the arduino_nano_rp2040 branch November 27, 2021 16:28
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Mar 13, 2023
@TheKitty
Merge pull request micropython#7669 from dhalbert/gif-delay-fix
763d7e4 
OnDiskGif delay was being chopped to 8 bits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimmo jimmo jimmo left review comments

@hoihu hoihu hoihu left review comments

@dpgeorge dpgeorge dpgeorge requested changes

Assignees
No one assigned
Labels
port-rp2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Board/Arduino Nano RP2040 Connect] Adding the first rp2 board with wifi.
6 participants
@iabdalkader @robert-hh @manpowre @dpgeorge @jimmo @hoihu
0