-
Notifications
You must be signed in to change notification settings - Fork 35
_GLOBAL constants #86
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
@bill88t Would you like to test this? You could see how much the .mpy gets smaller with this change. |
PR branch:
With
Main:
With
|
@BiffoBear Sorry, I merged your #81, which caused merge conflicts on this. |
Not to worry. I'll sort it out when I get up 😃 |
Does this need to be tested again post-merge from main? |
This change would provide a really great test for #197 on cookie cutter for testing the size of mpy compiled files from an automated action task. |
# Conflicts: # adafruit_wiznet5k/adafruit_wiznet5k_dhcp.py # adafruit_wiznet5k/adafruit_wiznet5k_socket.py
I tested this partially successfully with a Feather ESP32-S2. For these 3 examples:
Everything is working as expected with this modified version of the library. However on this example
This does look like it's potentially to do with some of the renamed constants from this PR, but I'm not certain. The main branch of this library does execute the same example without raising an exception. I think we'll want to figure out why this one behaves different and ideally try to make not raise the exception so that it'll behave the same as currently released library. |
Thanks for the review @FoamyGuy. You're right. I'll take a look at this and push a fix. Cheers, |
I should have mentioned as well: That exception comes after the server receives some data. These were my steps:
|
…in adafruit_wiznet5k_wsgiserver.py.
Hi, I've fixed the problem and your Telnet test now works on my system, the data is printed in the Mu serial and echoed back on my Telnet session. Hope it works for you too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Tested successfully on the latest version with Feather ESP32-S2 TFT.
Thanks for the memory improvement!
Updating https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k to 1.13.3 from 1.13.1: > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#82 from BiffoBear/NTP_fix_infinite_loop > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#86 from BiffoBear/_GLOBAL_constants Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
closes #84 Added
_
to all global constants not referenced by other modules.