-
Notifications
You must be signed in to change notification settings - Fork 74
Fixed MAC reverse byte order issue within the ESP32SPI library as it … #66
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
Fixed MAC reverse byte order issue within the ESP32SPI library as it … #66
Conversation
…was originally echoing the MAC address in reverse byte order. Added portable MAC function call within WSGISERVER.PY example to properly echo MAC address when running example.
@ladyada ganbatte done it :) |
@brentru wanna try this and use an external tool to verify the MAC of the esp32 |
Minor point, but can we mark this as a breaking change in the release notes? Some people (like me) may have code that uses the old order. |
why would it be a breaking change? usually you display the MAC, but code doesnt stop working because of the new order |
I suppose it's not breaking within CP (and perhaps dumb on my part), but for example, I use an ID based on old order to ID the ESP unit to my server. |
whatever y'all want to do. they're just numbers |
I don't want to break anyone's code. What I will do is add a new function to the adafruit_esp32spi.py library called MAC_address_actual to which you as the community can have a review on. |
…ibrary calling the actual MAC address and providing a usage of the function in the webserver code within WSGISERVER.
…ibrary calling the actual MAC address and providing a usage of the function in the webserver code within WSGISERVER. This time with line breaks at end to not break Travis.
For what its worth, it appears that the nina-fw is purposely sending the order this way, and the arduino WiFiNina library doesn't do anything to adjust it. In the example usage you can see they're reversing the output of the call when printing it, but the byte array the I'm not sure why this is the way it is. Does it make sense to simply document this behavior, and perhaps implement a helper method that reverses it for use when printing? |
i think reversing it in the example is also fine. |
@brentru Can you test this if you did not already? It looks like it's otherwise ready to merge. Thanks! |
@kattni I'll test later today. Assigning myself.... |
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.
The esp32spi.py
has not been updated since a349bac. I am not sure why GitHub is not throwing a merge conflict but newer methods like get_time do not exist within this file.
@brentru I believe it's possible that since the changes don't conflict with that part of the file, merging this PR will not have any problems. If you're satisfied with the changes, I say we merge it and verify that the newer functionality is still in the file. We can always revert it if necessary. |
@kattni Sounds good, I have no problems with the changes and would be OK to merge. |
@brentru Please merge it and verify the functions in question are still present. I will release the library if you give the go ahead. |
I verified there were no merging conflicts. Also tested the changes afterward. The new property and edited example are working as expected. As a sanity check I also tested the updated version with an example unrelated to this change and everything checked out fine there as well. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_ATECC to 1.0.5 from 1.0.4: > Merge pull request adafruit/Adafruit_CircuitPython_ATECC#9 from fgallaire/master Updating https://github.com/adafruit/Adafruit_CircuitPython_BluefruitSPI to 1.0.5 from 1.0.3: > Merge pull request adafruit/Adafruit_CircuitPython_BluefruitSPI#14 from jerryneedell/jerryn_unpack Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 3.2.0 from 3.1.2: > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#66 from mytechnotalent/ESP32-MAC-order-library-fix Updating https://github.com/adafruit/Adafruit_CircuitPython_PyBadger to 2.1.2 from 2.0.0: > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#26 from kattni/qr-code-update > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#25 from kattni/rename-show > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#24 from kattni/pronoun-badge
…was originally echoing the MAC address in reverse byte order. Added portable MAC function call within WSGISERVER.PY example to properly echo MAC address when running example.