ESP32S2: Support for CountIO

[microdev1]

No description provided.

[tannewt]

Code looks great! The PCNT looks really useful. I think you'll want to sort out sharing it's units.

[tannewt]

Just a couple minor things! Looks good overall.

[hierophect]

This looks great! Just a couple minor port organization comments, if we could get this all moved into a peripherals/pcnt.c file instead of the port root that'd be good. Do you have a good test sketch I can run with this to try it out?

[microdev1]

@hierophect Thanks for the review. I am using the following test sketch.

import countio
import time
import digitalio
from board import *

#Connect IO21 to IO14

led = digitalio.DigitalInOut(IO21)
led.direction = digitalio.Direction.OUTPUT

pin_counter = countio.Counter(IO14)

while True:
    led.value = True
    time.sleep(0.25)
    led.value = False
    time.sleep(0.25)
    print(pin_counter.count)

[hierophect]

Close! It's a minor change, but please change pcnt_handler to pcnt.c to match the naming conventions we use for peripherals/ source files.

[microdev1]

@hierophect A file named pcnt.c already exists in the esp-idf.

[jepler]

Does this cause a problem? In CircuitPython, for instance, there are many files named __init__.c and it's okay.

[microdev1]

No problem as such. My aim is to avoid confusion.

[hierophect]

My first impression would be to avoid giving peripherals files a prefix so that the naming is in line with the other ports, but the ESP-IDF is somewhat unique among HALs in that it doesn't have a prefix on its own source files (as compared to hdl_adc.h, nrfx_adc.c, stm32fxxxxxx_adc.c). Either way is fine with me as long as we're consistent.

[hierophect]

Chatted with @tannewt briefly on discord, let's keep the "pcnt.c" style for simplicity and consistency. The makefile enforces more extended include paths to avoid this kind of confusion anyway.

[hierophect]

Looks good!

[tannewt]

I'd prefer a return instead of filling out a field in an input struct.

[tannewt]

Thank you!

[tannewt]

One failing check due to network issues. All is good.