UMFEATHERS2 - implement use of DotStar for status led

[jerryneedell]

addresses #3922

For this PR, I used the common-hat/microcontroller/Pin.c from the atmel-samd port as a guide.
Hopefully I did not miss anything.

As noted int #3922 at power-on, the LDO2 pin is High but set as an INPUT
The PR does not change that. Is that OK?

The DotStar does reflect the status of CP -- blinking green before REPL -- Green in code.py and White in REPL
It is very dim, but useable.

I have run this on a umfeathers2 to verify the DotStar function.
I also created builds for the metros2 and ragtag to see if anything unexpected happened there. Nothing noted.

[UnexpectedMaker]

@jerryneedell LDO2 needs to be set to OUTPUT - See my comment in your issue about why the LED is dim with it being an input:
#3922 (comment)

[jerryneedell]

@UnexpectedMaker Hmm -- setting that to OUTPUT will make it very unique to this board. Where would you suggest doing that? Should it also be enabled?

[UnexpectedMaker]

@UnexpectedMaker Hmm -- setting that to OUTPUT will make it very unique to this board. Where would you suggest doing that? Should it also be enabled?

This board is unique in that is has a second LDO and power control over the APA via it.
I can't advise where this should be set, sorry. Maybe @tannewt can guide you.

[jerryneedell]

@UnexpectedMaker Hmm -- setting that to OUTPUT will make it very unique to this board. Where would you suggest doing that? Should it also be enabled?

This board is unique in that is has a second LDO and power control over the APA via it.
I can't advise where this should be set, sorry. Maybe @tannewt can guide you.

It may not be necessary if we don't need to enable it.

[jerryneedell]

This can wait until @tannewt and/or @hierophect can look at it and comment.
It may not even be something that should be implemented. It is useful for some testing I am doing but may not be desirable for general use.

It was a great learning experience for me

[UnexpectedMaker]

Not enabling the second LDO and relying on parasitic power via the CLK/DATA to make it light up is not really a solution I'm ok with. If there is no way to do this "at the board level" then use of the APA for this purpose should not be added. But none of this is really up to me, so getting feedback from the CPY team is going to be required :)

[jerryneedell]

If we want to implement this, I think we can add an "#ifdef CIRCUITPY_LDO2" block here https://github.com/adafruit/circuitpython/blob/main/supervisor/shared/rgb_led_status.c#L114
and set the LDO2 pin to an output and enable it.
Then in the mpconfogboard.h file for te umfeathers2 it can be enabled if desired.

[tannewt]

Not enabling the second LDO and relying on parasitic power via the CLK/DATA to make it light up is not really a solution I'm ok with. If there is no way to do this "at the board level" then use of the APA for this purpose should not be added. But none of this is really up to me, so getting feedback from the CPY team is going to be required :)

This should be doable at the board level. You can turn the second LDO on in board_init and turn it off in board_deinit. The tricky part will be if you want user control of it. pin sharing is implemented for the RGB status pins but it isn't elegant.

[jerryneedell]

Not enabling the second LDO and relying on parasitic power via the CLK/DATA to make it light up is not really a solution I'm ok with. If there is no way to do this "at the board level" then use of the APA for this purpose should not be added. But none of this is really up to me, so getting feedback from the CPY team is going to be required :)

This should be doable at the board level. You can turn the second LDO on in board_init and turn it off in board_deinit. The tricky part will be if you want user control of it. pin sharing is implemented for the RGB status pins but it isn't elegant.

@tannewt That seems like the right place to do this, but I think the intent is also for the user to be able to turn it off. @UnexpectedMaker Can you comment on that?

Would enabling LDO2 in board_init() prevent the user from turning it off?

Do you think this is worth implementing? I have found it very useful to have the status report from the LED - especially when using deep sleep since it is usually not connected to the REPL. If it crashes it is really nice to see where.
But if this will compromise user control of LDO2 then it may not be desirable.

What are your thoughts?

[tannewt]

You could enable sharing by copying how apa102_mosi_in_use works with pin in use checking.

[hierophect]

Unless I'm misunderstanding something, I don't see any reason that this pin (pin_GPIO21?) cannot be set to a different pin output default in board_init, and then accessed later by a user. If you set the pin to an output in board_init using the IDF functions, and add it to the never_reset array, it should be able to hold its value across resets without being part of the "claimed_pin" array that would prevent users from accessing it.

You have to use the IDF functions directly, because there's no DigitalIO object, but I don't think that should be an issue? If the IDF has changed the output for a pin, shouldn't be anything stopping them from just making that same pin into a DigitalIO and either turn it off or giving themselves total manual control. Then next time the board resets, it'll run board_init again, and return the pin to an output state. But you might want to put in a pin reset before changing things in board_init just to be careful.

[jerryneedell]

Unless I'm misunderstanding something, I don't see any reason that this pin (pin_GPIO21?) cannot be set to a different pin output default in board_init, and then accessed later by a user. If you set the pin to an output in board_init using the IDF functions, and add it to the never_reset array, it should be able to hold its value across resets without being part of the "claimed_pin" array that would prevent users from accessing it.

You have to use the IDF functions directly, because there's no DigitalIO object, but I don't think that should be an issue? If the IDF has changed the output for a pin, shouldn't be anything stopping them from just making that same pin into a DigitalIO and either turn it off or giving themselves total manual control. Then next time the board resets, it'll run board_init again, and return the pin to an output state. But you might want to put in a pin reset before changing things in board_init just to be careful.

@hierophect Can you point me to any examples of using the IDF functions within CircuitPython?

[hierophect]

@hierophect Can you point me to any examples of using the IDF functions within CircuitPython?

In this case you would draw directly from what's used in DigitalIO. common_hal_digitalio_digitalinout_switch_to_output sets the value of the pin and changes the output mode, and the IDF functions it calls are:

#include "components/driver/include/driver/gpio.h"
#include "components/hal/include/hal/gpio_hal.h"

gpio_set_level(pin_GPIO21.number, value);
gpio_set_direction(pin_GPIO21.number); //Mode is something like GPIO_MODE_DEF_OUTPUT

(note those IDF files you'll need to include)

I don't think this would be really any more janky than the kind of screen setup stuff we put in board_init already? But I could be wrong.

[jerryneedell]

@hierophect Can you point me to any examples of using the IDF functions within CircuitPython?

In this case you would draw directly from what's used in DigitalIO. common_hal_digitalio_digitalinout_switch_to_output sets the value of the pin and changes the output mode, and the IDF functions it calls are:

#include "components/driver/include/driver/gpio.h"
#include "components/hal/include/hal/gpio_hal.h"

gpio_set_level(pin_GPIO21.number, value);
gpio_set_direction(pin_GPIO21.number); //Mode is something like GPIO_MODE_DEF_OUTPUT

(note those IDF files you'll need to include)

I don't think this would be really any more janky than the kind of screen setup stuff we put in board_init already? But I could be wrong.

Would it be necessary to do anything to revert these settings in board.deinit()

[jerryneedell]

@hierophect I tried adding the following to board_init()

    // LDO2
    common_hal_never_reset_pin(&pin_GPIO21);
    // Set LDO2 to output and enable
    gpio_set_direction(pin_GPIO21.number, GPIO_MODE_DEF_OUTPUT);
    gpio_set_level(pin_GPIO21.number, true);

If I then try to access the board.LDO2 pin from the REPL I get

Adafruit CircuitPython 6.1.0-rc.0-14-g6976ec6ef-dirty on 2021-01-08; FeatherS2 with ESP32S2
>>> import digitalio,board
>>> ldo2 = digitalio.DigitalInOut(board.LDO2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: LDO2 in use
>>> 

This is due to :
https://github.com/adafruit/circuitpython/blob/main/ports/esp32s2/common-hal/microcontroller/Pin.c#L137

Is it necessary to put it in the never reset list?
If I don't put it int he never_reset list it seems to work OK and I can configure it via the REPL.

[tannewt]

Then next time the board resets, it'll run board_init again, and return the pin to an output state. But you might want to put in a pin reset before changing things in board_init just to be careful.

board_init is only called on start up or after deep sleep. It isn't called when the vm is reset. So you'll need custom "set it back on pin reset" code.

[jerryneedell]

Then next time the board resets, it'll run board_init again, and return the pin to an output state. But you might want to put in a pin reset before changing things in board_init just to be careful.

board_init is only called on start up or after deep sleep. It isn't called when the vm is reset. So you'll need custom "set it back on pin reset" code.

Do we want this to be reset or do we want it always to be set to an OUTPUT and enabled in board_init?
I have it being enabled in board_init and I can verify that the 3v3O pin goes to 3.3V ( it is ~2.6V without these changes) once the board is reset - hard or soft.

[hierophect]

If you don't put it in the never_reset list, the pin will be returned to an INPUT state whenever the board reset is called, which happens every time the vm resets.

I don't know why pin_is_free is checking the never_reset tables, that is nonstandard port behavior and is not shared for the STM32, NRF or SAMD ports. It's not something we ever formally hashed out though. I'd recommend the (never_reset_pins[offset] & mask) part of that line should actually be deleted in Microcontroller unless @tannewt has objections - I think cases like were one of the original reasons we opted to keep claimed_pins and never_reset_pins separate, IIRC.

[hierophect]

Just to recap on the overall strategy here, since this terminology might be getting confusing:

• the never_reset_pins array is a convention for every port. It is an array that marks IO pins that should never be reset to a default state, such as display pins, debugging pins, and other system pins. Quite literally it just keeps pins from being affected by a function called reset_all_pins called on VM resets.
• the claimed_pins array is also a soft convention. It is an array that marks pins that have been "claimed" by claim_pin. Modules call claim_pin when they construct and release pins when they deinit. It stops you from claiming pins that are in use in the python code.

Strategy here:

• Board init is called at MCU startup. We change the default behavior of the pin, without actually making a DigitalIO object. This means that the pin isn't "claimed" in the DigitalIO module, but the change also isn't recorded anywhere. We also add it to the never_reset array.
• When the board is reset, the tick in never_reset prevents the pin from being turned back into an INPUT
• A user can define the pin as a DigitalIO. This claims the pin, and does not change never_reset status. They can then turn the pin to whatever they want, and it will still not be modified by resets. However, if they run deinit on the pin, it will be removed from the claimed pins AND never reset, and will just act like a regular IO pin.
• Until the board is power cycled or woken from deep sleep, it will not return to the default set in board_init, since that default is not stored anywhere that can be accessed on a VM reset (I think this is fine?)

[jerryneedell]

I pushed my latest changes, let me know if more needs to be done.
The DotStar is still very dim for the status message -- I'm not really sure why but I get the same thing when setting it manually .
The status r,g,b level are only 30 of 255. But other boards with dotters (trinket) appear much brighter.
Looking into this.

[UnexpectedMaker]

I pushed my latest changes, let me know if more needs to be done.
The DotStar is still very dim for the status message -- I'm not really sure why but I get the same thing when setting it manually .
The status r,g,b level are only 30 of 255. But other boards with dotters (trinket) appear much brighter.
Looking into this.

Can you confirm the second LDO is outputting 3.3V with a multimeter?

[jerryneedell]

Yes. It is.

[UnexpectedMaker]

Yes. It is.

Ok, maybe you have one of the earlier FeatherS2's I shipped that had some "visually duller APAs". Prob not anything to worry about if LDO2 is confirmed as on.

[jerryneedell]

It is fine when set to higher values

[UnexpectedMaker]

It is fine when set to higher values

Yeah, I usually run them at 50%. I switched suppliers in November last year and the current APAs I'm using seem to be considerably brighter and a little richer in colour. Interestingly, under the microscope, the 2 APAs look identical, and I cant find any information from either supplier about why the previous reel were dimmer at the lower brightness levels. Hardware is fun ;)

[jerryneedell]

the colors are set to 0x30
https://github.com/adafruit/circuitpython/blob/main/supervisor/shared/rgb_led_colors.h#L5
brightness 50
https://github.com/adafruit/circuitpython/blob/main/supervisor/shared/rgb_led_status.c#L43
so the effective setting is more like 10 for each of r,g,b -- pretty low
https://github.com/adafruit/circuitpython/blob/main/supervisor/shared/rgb_led_status.c#L338

It is functional and very useful to see the status even if dim

[hierophect]

Does the APA102 have multiple manufacturers? My first thought would be clones, or legitimate licensees of the design that have variable quality control. I can't find anything online that verifies one way or the other though.

[jerryneedell]

At this point, I thin this is complete, unless you want something done in board_deinit() or if @tannewt has a concern with the change to the pin_is_free test... or anything else.

[hierophect]

LGTM

[tannewt]

I think there is a slight bug now because the user's pin state will leak on VM reset. I want do make this functionality more generic soon. So, this is good for now.