Initial support for RGB led as Status indicator, fixes #1382 #1925

C47D · 2019-06-05T02:28:27Z

Tested with custom board definition based on the Bast Pro Mini M0 (SAMD21).

Below is the mpconfigboard.h file, notice the CP_RGB_STATUS_R, CP_RGB_STATUS_G and CP_RGB_STATUS_B symbols, i connected there my RGB led, those pins were used because those have PWM capability on my board:

#define MICROPY_HW_BOARD_NAME "Electronic Cats Bast Pro Mini RGB M0"
#define MICROPY_HW_MCU_NAME "samd21e18"

#define MICROPY_PORT_A        (0)
#define MICROPY_PORT_B        (0)
#define MICROPY_PORT_C        (0)

#define CIRCUITPY_INTERNAL_NVM_SIZE 0

#define CP_RGB_STATUS_R	(&pin_PA15)
#define CP_RGB_STATUS_G (&pin_PA23)
#define CP_RGB_STATUS_B (&pin_PA06)

#define DEFAULT_I2C_BUS_SCL (&pin_PA08)
#define DEFAULT_I2C_BUS_SDA (&pin_PA09)

#define DEFAULT_SPI_BUS_SCK (&pin_PA17)
#define DEFAULT_SPI_BUS_MOSI (&pin_PA16)
#define DEFAULT_SPI_BUS_MISO (&pin_PA19)

#define DEFAULT_UART_BUS_RX (&pin_PA01)
#define DEFAULT_UART_BUS_TX (&pin_PA00)

#define BOARD_FLASH_SIZE (0x00040000 - 0x2000 - 0x010000)

#define IGNORE_PIN_PA03     1
#define IGNORE_PIN_PA12     1
#define IGNORE_PIN_PA13     1
#define IGNORE_PIN_PA20     1
#define IGNORE_PIN_PA21     1
// USB is always used.
#define IGNORE_PIN_PA24     1
#define IGNORE_PIN_PA25     1
#define IGNORE_PIN_PA30     1
#define IGNORE_PIN_PA31     1
#define IGNORE_PIN_PB01     1
#define IGNORE_PIN_PB02     1
#define IGNORE_PIN_PB03     1
#define IGNORE_PIN_PB04     1
#define IGNORE_PIN_PB05     1
#define IGNORE_PIN_PB06     1
#define IGNORE_PIN_PB07     1
#define IGNORE_PIN_PB08     1
#define IGNORE_PIN_PB09     1
#define IGNORE_PIN_PB10     1
#define IGNORE_PIN_PB11     1
#define IGNORE_PIN_PB12     1
#define IGNORE_PIN_PB13     1
#define IGNORE_PIN_PB14     1
#define IGNORE_PIN_PB15     1
#define IGNORE_PIN_PB16     1
#define IGNORE_PIN_PB17     1
#define IGNORE_PIN_PB22     1
#define IGNORE_PIN_PB23     1
#define IGNORE_PIN_PB30     1
#define IGNORE_PIN_PB31     1
#define IGNORE_PIN_PB00     1

So far i check if those pins are free, if so i construct the pwmout object on three static pulseio_pwmout_obj_t variables and set the duty cycle to 0 and freq to 50000.

What i think i haven't been able to do is make the new_status_color function work properly, i'm taking the rgb (uint32_t) parameter and get the corresponding r, g and b values from it, then i calculate the equivalent porcentage on a uint16_t variable to i can then use it to set the duty cycle with common_hal_pulseio_pwmout_set_duty_cycle. I'm checking the behaviour with gdb but most of the times the rgb parameter is being optimized, so i have to check it seeing the value on the r0 register of the cpu.

I can only see the RGB LED turn on green at startup, after that it remains off.

…tatus

tannewt · 2019-06-06T17:23:51Z

It sounds like the output is being reset even though never reset is used. I'd suggest breaking on common_hal_pulseio_pwmout_set_duty_cycle to track that the correct values make it to the pwmout class. If that's true, then I'd make sure the TC and pins aren't accidentally reset.

C47D · 2019-06-06T17:34:49Z

Hi, thanks for the reply, i will debug it later today and let yall know any information i get. I tried to upload a video to YT for you to see it but i wasn't able to do so (until now).

I am not sure if i'm missing something on the other configuration files, my mpconfigboard.mk, board.c and pins.c files are the same as the Bast Pro Mini M0 board, should i modify something on those too?

C47D · 2019-06-09T20:00:36Z

Hi,

Thanks for your patience, I did a quick debug session for this and noticed a couple of things:

The common_hal_pulseio_pwmout_set_duty_cycle breakpoint is hit and then i continue but the LED takes a little while to lit or sometimes i doesn't lit at all, at the end of the attached log there's an example self=0x20001ca8 <rgb_status_r>, duty=12336) but the red LED remains off.
Is this somewhat expected?
When the common_hal_pulseio_pwmout_set_duty_cycle breakpoint is hit i do a backtrace command and the new_rgb_color appears to have two parameters, when it only have one:

(gdb) 
Continuing.

Breakpoint 2, common_hal_pulseio_pwmout_set_duty_cycle (
    self=0x20001ca8 <rgb_status_r>, duty=0) at common-hal/pulseio/PWMOut.c:324
324	extern void common_hal_pulseio_pwmout_set_duty_cycle(pulseio_pwmout_obj_t* self, uint16_t duty) {
(gdb) bt
#0  common_hal_pulseio_pwmout_set_duty_cycle (self=0x20001ca8 <rgb_status_r>, 
    duty=0) at common-hal/pulseio/PWMOut.c:324
#1  0x000244e6 in new_status_color (rgb=<optimized out>, rgb=<optimized out>)
    at ../../supervisor/shared/rgb_led_status.c:206
#2  0x000039a0 in rgb_led_status_init ()
    at ../../supervisor/shared/rgb_led_status.c:145
#3  main () at ../../main.c:396

then I'd make sure the TC and pins aren't accidentally reset

Sorry for the dumb question, how can i do this?

rgb_status_log.txt

tannewt · 2019-06-10T23:52:44Z

Try commenting out the -flto line in the makefile and doing it again. (only samd51 may have the space for it though.)

I use this https://github.com/bnahill/PyCortexMDebug to inspect the TC registers and the PORT to check the mux settings.

C47D · 2019-06-11T00:38:21Z

I only got a SAMD21 device and if I comment out the -flto line the flash isn't enough :/.

I will check the repo, thanks for the link.

tannewt · 2019-06-11T19:40:39Z

@C47D Please email me, scott@adafruit.com and we'll get you more boards. Thanks!

C47D · 2019-06-11T20:24:34Z

Just did @tannewt :-)

it's pins.

tannewt · 2019-07-06T20:02:29Z

Ok, I think the two remaining things are:

Add define for whether the RGB signals should be inverted.
Adjust the duty cycle for the current brightness.

…signals.

C47D · 2019-07-06T21:51:52Z

In fae1039 I have added a new symbol CP_RGB_STATUS_INVERTED_PWM to know when the PWM signal should be inverted. Should i also invert status_rgb_color values in clear_temp_status?

tannewt · 2019-07-08T01:13:30Z

Yes please. It should be inverted any time it is written to the PWM.

C47D · 2019-07-08T02:22:42Z

In regard of adjusting the colors brightness, i think it's enough just to add CP_RGB_STATUS_LED in the #if defined on color_brightness and set_rgb_status_brightness. Like so:

uint32_t color_brightness(uint32_t color, uint8_t brightness) {
    #if defined(MICROPY_HW_NEOPIXEL) || (defined(MICROPY_HW_APA102_MOSI) && defined(MICROPY_HW_APA102_SCK)) || (defined(CP_RGB_STATUS_LED))
    uint32_t result = ((color & 0xff0000) * brightness / 255) & 0xff0000;
    result += ((color & 0xff00) * brightness / 255) & 0xff00;
    result += ((color & 0xff) * brightness / 255) & 0xff;
    return result;
    #else
    return color;
    #endif
}

void set_rgb_status_brightness(uint8_t level){
    #if defined(MICROPY_HW_NEOPIXEL) || (defined(MICROPY_HW_APA102_MOSI) && defined(MICROPY_HW_APA102_SCK)) || || (defined(CP_RGB_STATUS_LED))
    rgb_status_brightness = level;
    uint32_t current_color = current_status_color;
    // Temporarily change the current color global to force the new_status_color call to update the
    // LED. Usually duplicate calls of the same color are ignored without regard to brightness
    // changes.
    current_status_color = 0;
    new_status_color(current_color);
    #endif
}

tannewt · 2019-07-08T21:08:02Z

Yup! Looks like it.

C47D · 2019-07-09T16:42:26Z

Done!, you should add CP_RGB_STATUS_INVERTED_PWM to the particle xenon mpconfigboard.h file to get the PWM signals inverted.

tannewt

Looks great! Thank you!

[supervisor/shared/rgb_led_status.c] Initial support for RGB led as S…

9de46f3

…tatus

C47D changed the title ~~[supervisor/shared/rgb_led_status.c] Initial support for RGB led as S…~~ [DRAFT - supervisor/shared/rgb_led_status.c] Initial support for RGB led as Status Jun 5, 2019

C47D changed the title ~~[DRAFT - supervisor/shared/rgb_led_status.c] Initial support for RGB led as Status~~ [DRAFT] Initial support for RGB led as Status indicator Jun 6, 2019

C47D changed the title ~~[DRAFT] Initial support for RGB led as Status indicator~~ [DRAFT] Initial support for RGB led as Status indicator, fixes #1382 Jun 6, 2019

Fix up nrf so that it is initialized properly. Also, do not reset

5610e05

it's pins.

[rgb led status] Add CP_RGB_STATUS_PWM_INVERTED symbol to invert pwm …

fae1039

…signals.

[rgb status] Also invert the pwm values on clear_temp_status

324cbe4

[rgb status] Adjust to current brightness

40fbac1

tannewt self-requested a review July 9, 2019 17:02

tannewt changed the title ~~[DRAFT] Initial support for RGB led as Status indicator, fixes #1382~~ Initial support for RGB led as Status indicator, fixes #1382 Jul 9, 2019

tannewt approved these changes Jul 9, 2019

View reviewed changes

tannewt merged commit 6fad383 into adafruit:master Jul 9, 2019

C47D deleted the rgb_status branch July 9, 2019 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Initial support for RGB led as Status indicator, fixes #1382 #1925

Initial support for RGB led as Status indicator, fixes #1382 #1925

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Initial support for RGB led as Status indicator, fixes #1382 #1925

Initial support for RGB led as Status indicator, fixes #1382 #1925

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!