Skip to content
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

Expose more NINA FW SPI command handlers: getCurrBSSID, getIdxBSSID, getIdxChannel #74

Closed
anecdata opened this issue Sep 20, 2019 · 8 comments

Comments

@anecdata
Copy link
Member

anecdata commented Sep 20, 2019

These handlers are useful for general LAN troubleshooting, especially in networks with multiple APs having the same SSID (sometimes called 'roaming networks').

I have working code, will test more then submit a PR.

NINA FW SPI command handlers not defined or implemented in esp32spi.py after this PR:

EDIT: Found these later. Their counterparts are defined in esp32spi.py, but unimplemented:

  • getCurrEnct
  • getDataTcp
  • disconnect

Potential new command handlers:

I submitted an issue awhile back in the Arduino NINA repo to add a digitalRead command handler, but the devs closed it as "wontfix". There's also an open issue there for analogRead. Not sure why they closed digitalRead but left analogRead open.

Also not sure how much Adafruit wants its NINA fork to deviate from Arduino... could submit issues to the Adafruit NINA repo for the read handlers?

@ladyada
Copy link
Member

ladyada commented Sep 20, 2019

i'd be ok with them but ideally you could also add matching handlers to espspi :)

@anecdata
Copy link
Member Author

Yeah, that would be the idea. I could do the expose on the ESP32SPI side once NINA is available, but I'm not cpp enough to do the NINA side.

@ladyada
Copy link
Member

ladyada commented Sep 20, 2019

finding a buddy and working together sounds like a good idea!

@docmollo
Copy link
Contributor

@anecdata I'm happy to help out on the NINA fw side, if you'd like. This weekend is booked for me, and fair warning, work has been crazy lately, but the next few weekends are looking clear at the moment. We can work here, in the CP discord, private DM on Discord, or I can DM you my email address and we can work that way. Let me know....

@anecdata
Copy link
Member Author

anecdata commented Sep 21, 2019

@docmollo Cool, thank you! Probably similar to what you did with the enterprise handlers. I can work wherever too. Discord PM is fine, keeps the chatter from distracting others. I usually have Discord up when I'm around (and often when I'm not). Email's OK too. Issues and PRs will make it to Github.

It's separate from this present issue (which has a PR submitted to close it).

For background... there's an analogRead PR already in ESP32SPI ...could add the digitalRead to it explicitly or implicitly. One PR I would think on the ESP32SPI side, and one on the Adafruit NINA fork? There's no issue submitted for the Reads in the Adafruit NINA fork, not sure if that's needed or if the PR will suffice.

On the Arduino fork, just for history / completeness, they closed the digitalRead, but left open the analogRead. No idea why - maybe for battery voltage. Doesn't matter since we'll be (I assume) working on the Adafruit fork.

It's mainly useful for the Adafruit ESP32 Feather and the Adafruit ESP32 [Huzzah] Breakout, which both have tons of available pins just chilling there waiting to be appreciated :-)
https://gist.github.com/anecdata/80aa1b95933c0d37828c0c177f229d73
(or maybe some non-Adafruit NINA-compatible ESP32 boards)

@tannewt
Copy link
Member

tannewt commented Sep 23, 2019

I recommend public chatter. That way others can follow along as well. Feel free to chat in #circuitpython or I could add an #esp32spi channel.

@brentru
Copy link
Member

brentru commented Sep 23, 2019

@anecdata @docmollo I'd be more than happy to discuss changes or help out testing on the public discord, feel free to ping me Monday thru Thursday. on the #circuitpython channel or tag me on issues/pr's.

For background... there's an analogRead PR already in ESP32SPI ...could add the digitalRead to it explicitly or implicitly.

We have a digitalio class within ESP32SPI, is this what you mean by digitalRead? I think adding a separate analogio class which mocks CircuitPython's analogio would be a good way to implement this as it'd match the esp32spi digitalio implementation.

https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/master/adafruit_esp32spi/digitalio.py

@anecdata
Copy link
Member Author

anecdata commented Sep 25, 2019

Closing this issue, PR #75 addressing getCurrBSSID, getIdxBSSID, and getIdxChannel, has been merged and released. Thanks @brentru for review and test! Any additional command handlers should be addressed in separate issues / PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants