search

adafruit / Adafruit_CircuitPython_Wiznet5k

Pure-Python interface for WIZNET 5k Ethernet modules
Other
15 stars 37 forks source link

DHCP state machine error handling #80

Closed BiffoBear closed 1 year ago

BiffoBear commented 1 year ago

Closes #75 #79. @FoamyGuy Refactored DHCP.parse_dhcp_response to raise ValueError instead of returning 0, 0. Refactored DHCP._dhcp_state_machine to catch the exception, print it if debugging is on, and continue.

Fixed off-by-one bugs in DHCP.send_dhcp_message that caused the buffer to grow by 1 or 2 bytes every time the function is called. Fixed logic bugs in DHCP.parse_dhcp_response that meant that some errors were not being caught. Fixed a bug in DHCP.parse_dhcp_response where an IP address with more than 4 bytes would be returned if more than one router address was supplied by the DHCP server.

FoamyGuy commented 1 year ago

Thanks for working on this! I will check it out this evening.

BiffoBear commented 1 year ago

I tested this successfully on a Feather S2 TFT under both 7.3.3 and 8.0.0 beta5. Can confirm this fixes the issues mentioned in #75.

It's now able to reconnect successfully to my wired network. I'm guessing one of the off-by-1 errors that were fixed perhaps was the root cause of my transaction ID problem. Not certain, but either way it does appear to be solved, I have been able to successfully run the simpletest several times under both versions of circuitpython core.

That's good news. Thanks for testing this. The 'f' you were seeing with the error shows that the problem you had was a mismatched packet ID, probably from another device's DHCP request. If you run with debug=True I expect you'll see occasional messages about that. I never see that here because there is very little DHCP traffic on my network. The off-by-ones all caused errors to be ignored.

FoamyGuy commented 1 year ago

I merged main into this to resolve the current conflicts.

Going to merge this PR. I re-tested the simpletest successfully with Feather TFT from the branch after merging main and resolving conflicts.

For my ethernet environment the fixes in this PR seem to be required in order for me to successfully connect to the network. So merging this one will allow me to test out the other open PRs.