caksoylar / zmk-rgbled-widget

A ZMK module to add battery & BT indicators using an RGB LED (like in Xiao BLEs)
MIT License
82 stars 11 forks source link

Configurable LED brightness #3

Closed diegolhambi closed 1 month ago

diegolhambi commented 1 month ago

Is it possible to change LED brightness using PWM or something similar?

caksoylar commented 1 month ago

In theory I think it should be, currently we use the GPIO LEDs implementation so it is on/off only. I think it'd be nice if the implementation would work with both types and we define PWM LEDs for the supported boards along with brightness setting support. I hadn't looked into it before because it hasn't seemed necessary but I am happy to look into it when I have a chance or have someone else contribute it.

diegolhambi commented 1 month ago

I tried to fiddle around the code, but I don't understand C and Zephyr very well. 😞

From what I searched the internal LEDs from XIAO can be used with PWM, it is correct?

caksoylar commented 1 month ago

I believe so. I see your attempts at https://github.com/caksoylar/zmk-rgbled-widget/compare/main...diegolhambi:zmk-rgbled-widget:main which seems mostly correct but I don't see where you define pwm_led* nodes that you refer to. I assume they'll need to be defined in the pinctrl node of the board, like mentioned in https://zmk.dev/docs/features/backlight#multiple-backlight-leds. And seeeduino_xiao_ble board doesn't define them by default here.

I think you also want only one pinctrl node &pwm_led, then you can refer to indices n under it with pwms = <&pwm0 n PWM_MSEC(10) PWM_POLARITY_NORMAL>; etc., like documented in the backlight page.

If it helps, there is a PWM sample in Zephyr showing the code API and board overlays as well.

diegolhambi commented 1 month ago

Thanks for the info, you helped a lot!

I've been able to make work declaring the pwm-leds in the overlay, however the red LED was always light up so I put a initialization resetting the brightness to 0 in the led_init_thread and worked as intended.

I found one bug, soon after powering up the board, all LEDs light up before the indicate_battery kick in and I don't have clue why. 😆

caksoylar commented 1 month ago

That's great to hear 😄

Regarding the bad initialization, looking at this it looks like you might want to set nordic,invert; in the pinctrl node since we want the pin to be high for the LEDs to be disabled: https://docs.zephyrproject.org/latest/build/dts/api/bindings/pinctrl/nordic%2Cnrf-pinctrl.html

diegolhambi commented 1 month ago

Setting nordic,invert; fixed the bad initialization 💯.

I removed the "reset" in the led_init_thread too.

diegolhambi commented 1 month ago

I think I have made something like what you described.

#if IS_ENABLED(CONFIG_PWM)

#define LED_NODE_ID DT_COMPAT_GET_ANY_STATUS_OKAY(pwm_leds)

#else

#define LED_NODE_ID DT_COMPAT_GET_ANY_STATUS_OKAY(gpio_leds)

#endif

This will load the correct driver if the board have the PWM enabled.

caksoylar commented 1 month ago

Nice! As long as you also gate the brightness setting function calls with the #if/else, should be all good. A PR would be welcome.

diegolhambi commented 1 month ago

I'm going to close this PR because configuring PWM on the nRF activates an additional clock, and there are no effective gains in battery consumption.