Implement LED switching #5574
Implement LED switching #5574
Conversation
bieleluk
requested review from
obrusvit,
matejcik and
TychoVrahe
as code owners
|
| model | device_test | click_test | persistence_test |
|---|---|---|---|
| T2T1 |  test(all)  main(all)  |
 test(all)  main(all)  |
 test(all)  main(all)  |
| T3B1 |  test(all)  main(all)  |
 test(all)  main(all)  |
 test(all)  main(all)  |
| T3T1 |  test(all)  main(all)  |
 test(all)  main(all)  |
 test(all)  main(all)  |
| T3W1 |  test(all)  main(all)  |
 test(all)  main(all)  |
 test(all)  main(all)  |
Latest CI run: 17175285187
| return; | ||
| } | ||
|
|
||
| driver->enabled = enabled; |
There was a problem hiding this comment.
Should we turn off the LED here too? (similar to how it was done in other drivers)
There was a problem hiding this comment.
I would say yes. It follows the same structure as the haptic driver drv2625.c
There was a problem hiding this comment.
i agree, lets turn in off
|
@cepetr this PR touches some drivers code. Can you take a look? Thank you. |
bieleluk
force-pushed
the
bieleluk/led-switching
branch
from
4f37de2 to
9377ab3
Compare
|
|
||
| while (pm_turn_on() != PM_OK) { | ||
| rgb_led_set_color(0x400000); | ||
| #ifdef USE_RGB_LED |
There was a problem hiding this comment.
if we don't use LED, we don't need the delays either
|
|
||
| // Set RGB LED color | ||
| // color: 24-bit RGB color, 0x00RRGGBB | ||
| void rgb_led_set_color(uint32_t color); | ||
|
|
||
| #define RGBLED_GREEN 0x040D04 |
There was a problem hiding this comment.
we can probably keep this for now but these values seem entirely UI-specific, so it should not be defined at driver level. i am thinking that the led blinking in bootloader should be seprate rust UI function exposed to bootloader
| @@ -364,6 +364,7 @@ pub trait FirmwareUI { | |||
| device_name: Option<TString<'static>>, | |||
| paired_devices: Vec<TString<'static>, 1>, | |||
| auto_lock_delay: TString<'static>, | |||
| led: Option<bool>, | |||
There was a problem hiding this comment.
the parameter name isn't very helpful
| rgb_led_set_color(RGBLED_YELLOW); | ||
| #endif | ||
| systick_delay_ms(1000); |
There was a problem hiding this comment.
no need for delay without led
| return; | ||
| } | ||
|
|
||
| driver->enabled = enabled; |
There was a problem hiding this comment.
i agree, lets turn in off
[no changelog]
bieleluk
force-pushed
the
bieleluk/led-switching
branch
from
9377ab3 to
437a492
Compare
Enable LED switching off, similar to the haptic feedback.
Required by Figma
LED driver:
enabledfield in the driver structrgb_led_set_enabledrgb_led_get_enabledDevice menu
show_device_menuFwUI functionDeviceMenuResultvariantSyscalls
SYSCALL_RGB_LED_SET_ENABLEDSYSCALL_RGB_LED_GET_ENABLEDPython
rgb_ledwithrgb_led_set_enabledfunctionUSE_RGB_LEDStorage
set_rgb_ledandget_rgb_led()functionsTODO