Implement Digital Read & Analog Read counterparts to proposed NINA FW command handlers

[anecdata]

@docmollo, @brentru, @tannewt, @ladyada

I'm moving the discussion here for the ESP32SPI side of handling analog and digital reads from ESP32 hardware in the context of CircuitPython ESP32SPI. This issue consolidates #46 (which could be closed since it's subsumed under this one), and the later discussions in #74 (read there for more background on this issue) which were only tangentially related to that issue and will (hopefully) be closed soon (PR submitted).

The present enhancement proposal is to:

1. Implement setDigitalRead() and setAnalogRead() command handlers in the Adafruit fork of the NINA firmware. setPinMode() already exists. setDigitalWrite() and setAnalogWrite() also already exist. An issue or PR needs to be expanded or opened on the NINA side for this.
2. Implement set_digital_read() and set_analog_read() in adafruit_esp32spi.py. set_pin_mode() is already implemented. set_digital_write() and set_analog_write() are also already implemented.
3. Expand the ESP32SPI digitalio.py module for digital reads.
4. Create an ESP32SPI analogio.py module for analog writes and reads.

Implementation could be roughly in this order. Complete low-level functionality would become available with items 1 and 2 above. Then, expanding ESP32SPI digitalio and adding ESP32SPI analogio would provide higher-level class interfaces analogous to Circuitpython digitalio and analogio usage.

[brentru]

@anecdata Your implementation plan looks solid, let me know if you'd like assistance or code reviews. Looking fwd. to testing!

[anecdata]

Just noticed that, unlike 16-bit core CP analogio, the analog write in esp32spi is 8-bit From a cursory check, PWM (all? GPIO pins) and DAC (pins IO25/DAC1 & IO26/DAC2 on the breakout, A0/DAC2 and A1/DAC1 on the Feather) are both 8-bit, so this makes sense. Not sure if analogWrite() actually uses DAC on those pins though.

I believe ESP32 analog input is 12-bit default. Presumably we want to support the full range?

ADC#2 can't be used while wi-fi is running (virtually always in the context of esp32spi). Only ADC#1 can be used while wi-fi is running (pins IO32/A7, IO33/A9, I34/A2, I35/A13, I36/A4, I39/A3 on the Adafruit HUZZAH32 Breakout Board and ESP32 Feather; I35/A13 is the resistor-divided VBAT). See here, here, and here.

[anecdata]

There seems to be only INPUT and OUTPUT in NINA, no facility for internal Pull. That's fine. (Using Arduino...) Pins 32 & 33 float at a low voltage, 35 floats with unconnected battery. 34, 36, and 39 are 0.00 on this Feather for whatever reason. Watching batteries charge is only slightly more interesting than watching paint dry.

[docmollo]

@anecdata So I've poked through the NINA fw....I might have bitten off more than I can chew by volunteering to help you ;)

I also re-read Issue #74 where I had originally volunteered. I saw what you were talking about on the Arduino fork of the NINA fw. On the upside, they already have the code to in their library for the digitalRead...but they don't have any library code to support the analogRead. I did see the rough code on arduino/nina-fw#23 ...that might work, but I haven't tried it.

I should mention, I'm not a programmer/coder/software engineer...I just hack on code. Heh... I spent most of today trying to understand how and why they did the things they did in the current NINA firmware...and...wow. It hurts my head.

So all that said, I think I should be able to get the digitalRead code put together this week and have something for you to test against on the CP side. The rest...well, I'll try but no promises ;)

On a mostly unrelated side note, I did see that they're finally starting to work on the WPA2 Enterprise code: arduino/nina-fw#33. This is cool, since they're supporting some functionality I have no way to test or validate (ie, certificates). I'm going to keep an eye on that and work on merging it into the Adafruit fork when it's ready.

[anecdata]

@docmollo Gotcha, thanks. I’m not a pro at this either. The layers of abstraction and even how some of the data types get used in some of the C libraries makes my head spin.

I got back into Arduino after a couple of years to make a little test bed for this, and the low level esp analog stuff gets thick with calls for resolution, attenuation, etc. I don’t think we need to go that deep if Arduino library level pieces are available (like the rough code in 23), but analogRead def looked more complex than I had initially imagined.

Anyway, sounds good to start with digitalRead. I really appreciate you volunteering on thIs. We’ll do what we can and seek help if we need. I want to understand the NINA side better too, so this will be a good learning experience.

[anecdata]

@docmollo I poked around a little, and I think I'm seeing what you pointed out above, that there's not any analog read in NINA like there is for digital read.

Analog write uses an LED Control PWM peripheral, so no DAC as I wondered above.

As far as I can tell, to do analog read, we would be down in the thick of the Espressif ADC API. Since we're not in the Arduino environment, I don't think we have access to analogRead() per se (like the rough code in 23) without merging it in somehow (I could be wrong, and I haven't looked at what would be involved there, or if it's even possible - I know nothing about the NINA build process at this point). For an analogRead(pin) in Arduino on an ESP32, where is the underlying code coming from? Presumably from the code downloaded via the ESP32 Boards Manager, since I don't see any ESP32 support in Arduino (at least in their main Github repo)... and how does that relate to the code components of the NINA build? I have no idea at this point.

If we hit a roadblock on analog read, I wonder if it makes sense to try to get a read on the NINA main branch intentions for their open issue on analog read that you referenced before potentially doing a high-effort parallel invention of the wheel.

Addendum 1: NINA uses the Espressif IDF (Espressif IoT Development Framework), Arduino uses the Espressif Arduino core for the ESP32, a distinct framework. While there may be some useful concepts in the Arduino core, I suspect trying to use anything from there will be more complex. Since NINA uses IDF, that's probably the place to focus for more easily integrate-able functionality. Probably already obvious to readers of this thread, but I had to run it down for my own understanding.

Addendum 2: It appears there may be a way to incorporate the Arduino core into the IDF, though I think this may add unneeded size and complexity (e.g., dependencies) to the NINA FW. In addition, the Arduino core seems to implement analog read at a very low level, without the abstractions available in the IDF. I think what we want with analog read should be achievable with the basic IDF.

[anecdata]

See Addenda in the comment above. Analog read may be as streamlined as doing these three calls (and the first two merely assert the defaults). A more fleshed-out example has production-date-dependent calibration, and other calibration and noise reduction. I think it would be very complex to expose all of those capabilities, but with the raw read, we probably get 90% of the benefit, and users can multisample to their heart's content.

Should note also that there are strong statements not to use pins 36 and 39 since they are tied to the Hall Effect sensor, but it's not clear if they are OK to use if you don't intend to use the Hall Effect sensor. We may be able to determine that in testing, if nowhere else.

[brentru]

@anecdata I've updated nina-fw to ESP-IDF 3.3, new nina-fw releases will be built with ESP-IDF 3.3 going forward. Not sure how/if this impacts this issue, release notes are here.

[anecdata]

I have a working digitalRead NINA fw & ESP32SPI.

The build using IDF v3.3 ultimately worked (had never done it before). Some errors along the way, not sure if these are specific to my setup or relevant to others, but here they are fwiw:

• Using macOS 10.15... had to update command line tools, manually install some required python packages, and allow a whole bunch of components to access disk (new tighter permissions model in Catalina). Also ...nina-fw/build/app_trace/libappp_trace.a didn't automatically unarchive.

• esptool: had to remove the --before no_reset to get it to connect to Feather ESP32.

[jerryneedell]

@anecdata IIRC -- the --before no_reset is not used if you are directly connecting to a feather esp32 -- but it should be used for the passthrough cases. Also, when doing the direct connection to an esp32 I sometimes found it necessary to do and erase_flash before loading a new image. Sometimes the new image would not work when copied over an old one especially if I had MicroPython on the esp32 first.

[anecdata]

@jerryneedell I hadn't tried flashing over something different yet. Thanks for the no_reset clarification. I couldn't find the old instructions for directly flashing an ESP32 Feather or Huzzah breakout, most folks now I suppose doing some kind of passthru onto an all-in-one or over to an Airlift add-on. This change only makes sense on something with more pins exposed... ESP32 Feather, ESP32 Huzzah breakout, maybe even a tinyPICO.

[jerryneedell]

FYI -- the "old way" is here https://learn.adafruit.com/adding-a-wifi-co-processor-to-circuitpython-esp8266-esp32/program-with-esptool#programming-esp32-spi-firmware-with-esptool-4-7

[brentru]

@anecdata

I have a working digitalRead NINA fw & ESP32SPI.

Exciting! When you're ready to wrap/bundle, pls tag me on the PR so I can test the work!

[anecdata]

@jerryneedell "old" is the new "new" ;-)

@brentru I'm working on analog read, cleaner to release both, unless I get stuck. Haven't done much analog before, Even writing an analog value on ESP32 and reading it on SAMD51 isn't accurate to the 8-bit level of write precision, even with averaging multiple samples. Granted, this is breadboarded with jumper-antennae all over and there's been no calibration, but it seems so far that accurate A/D is challenging. Precision is not the problem.

[anecdata]

It seems right for analog read to return to CircuitPython with an integer containing a 16-bit value (an integer with a default 12-bit value comes back from the ESP32). This is in keeping with CircuitPython's analogio using integers with 16-bit ranges.

It should be noted that ESP32SPI set_analog_write() takes a float between 0.0 and 1.0 as parameter, and I suspect that's already built into some user code and maybe even a library or two.

Any objections to proceeding with intreturn from analog read?

[anecdata]

When this issue closes, #46 should also be able to close.

[brentru]

Close via https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/80