-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
stm32: Use lib/wiznet5k instead of drivers/wiznet5k. #9021
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
Conversation
Thanks for this @robert-hh and @omogenot. I was keen to see this happen, but between not having any stm&wiznet hardware to develop against and too many in-progress projects/pr's already it had fallen down my list. |
I dropped the last commit, now that PR #8974 is merged. |
Thanks, this is good. I've tested it on a PYBv1.0 with W5200 and it works. But the change to |
Also, I think the |
That was the part I was not sure about. Will you merge 9034 first?
Can do that. I kept it for now because I was not sure whether it was needed by a port. |
I have now merged #9034. So please rebase and rework this on top of that (should be easy). If you could also delete the old drivers as a separate commit, that would be great. Then CI can test it as well. |
7b5fce9
to
f2ea0c6
Compare
The PR is updated. I added the function mp_hal_get_spi_obj() to spi.c, but kept spi_from_mp_obj() is kept, because it is used by the 33ck driver, and I cannot test that. |
435933b
to
9f4a8be
Compare
@robert-hh
To set dns server address to the one provided by dhcp or fixed ifconfig, or use public Google DNS if none was given (a.k.a. the provided dns server address started by 0). I have some customers who do not allow Internet access to devices and provide their own internal dns servers instead. |
This is the first step. The code compiles.
The function spi_from_mp_obj() is kept, since it is used by the cc3k driver, which I cannot test, and so it is untouched.
- Add lib/wiznet5k into the 'make submodules' step. - Split the stm32 builds for wiznet5k and cc3k. - Run 'make .... clean' after making the wiznet5k build.
Of course I can add that. It may however also be a separate commit, since it is not related to moving the files. But in any case: how can I tell that it works? I had expected, that it is used by socket.getaddrinfo(). But it's not. |
@robert-hh |
To compile properly, it should be:
But without LWIP, the nic does not come up. So there is another problem in this PR. |
@robert-hh
But this is another story. |
False alarm. This config requires nic.active(True), while the previous set-up did not have the active() method at all. Yes, the changed wiznet5k_gethostbyname() is used and picks up the DNS address from the wiznet structure. I cannot check the actual DNS query with wireshark, since I do not own a hub or switch, which can be configured as hub. So I cannot see the traffic between the PyBoard and the router. |
Instead of the default 8.8.8.8. The change was suggested by @omogenot. This change could go into a separate PR, but since it is related to this wiznet5k topic, I added it here, avoiding another PR.
9f4a8be
to
aac8e0d
Compare
Codecov Report
@@ Coverage Diff @@
## master #9021 +/- ##
=======================================
Coverage 98.37% 98.37%
=======================================
Files 156 156
Lines 20424 20424
=======================================
Hits 20092 20092
Misses 332 332 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This PR uses lib/wiznet5k instead of drivers/wiznet5k, like the rp2040 port already does. The change was initiated by @omogenot, and I picked up the baton for test, bug fixing and completion. See the thread at issue #9013. I tested the code change with a Wiznet W5500 breakout, with and without LWIP support. The performance is equivalent to the W5500 support for RP2040.
One Note:
The last commit in this PR consists of the combined changes of PR #8974, so testing runs well. This last commit can either be removed, or the other PR gets obsolete.