pwm_set_mask_enabled missing complementary pwm_get_mask_enabled #3045

New issue

Open

opened 2026-06-29 18:23:27 -05:00 by sprickett · 1 comment

sprickett commented

2026-06-29 18:23:27 -05:00

(Migrated from github.com)

Adding a formal get function would complement the existing set function

uint32_t slice_mask = pwm_hw->en;  // Where is pwm_get_mask_enabled() when you want it?
slice_mask |= 1u << pwm_gpio_to_slice_num(RED_PIN);
slice_mask |= 1u << pwm_gpio_to_slice_num(GREEN_PIN);
slice_mask |= 1u << pwm_gpio_to_slice_num(BLUE_PIN);
pwm_set_mask_enabled(slice_mask);

Obviously one could just write:

pwm_hw->en |= slice_mask;

But formal get and set functions are perhaps nicer.

I notice that get functions are few and far between so there might be a reason why this is not here already

Adding a formal get function would complement the existing set function ```C uint32_t slice_mask = pwm_hw->en; // Where is pwm_get_mask_enabled() when you want it? slice_mask |= 1u << pwm_gpio_to_slice_num(RED_PIN); slice_mask |= 1u << pwm_gpio_to_slice_num(GREEN_PIN); slice_mask |= 1u << pwm_gpio_to_slice_num(BLUE_PIN); pwm_set_mask_enabled(slice_mask); ``` Obviously one could just write: ```C pwm_hw->en |= slice_mask; ``` But formal get and set functions are perhaps nicer. I notice that get functions are few and far between so there might be a reason why this is not here already

lurch commented

2026-06-30 06:51:45 -05:00

(Migrated from github.com)

Hmmmm... In hindsight, I wonder if it would have made more sense for pwm_set_mask_enabled to have an API more like pio_set_sm_mask_enabled i.e. a separate mask and enabled parameter, so that your example above would be:

uint32_t enable_mask = 1u << pwm_gpio_to_slice_num(RED_PIN) | 1u << pwm_gpio_to_slice_num(GREEN_PIN) | 1u << pwm_gpio_to_slice_num(BLUE_PIN);
pwm_set_mask_enabled(enable_mask, true);

We can't change the existing function's prototype because that would break backwards-compatibility, but maybe it's worth adding a new function for this? 🤔

Hmmmm... In hindsight, I wonder if it would have made more sense for `pwm_set_mask_enabled` to have an API more like [`pio_set_sm_mask_enabled`](https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#group_hardware_pio_1gaf726f77ee21f5373cadf064a4e0b4935) i.e. a separate `mask` and `enabled` parameter, so that your example above would be: ```c uint32_t enable_mask = 1u << pwm_gpio_to_slice_num(RED_PIN) | 1u << pwm_gpio_to_slice_num(GREEN_PIN) | 1u << pwm_gpio_to_slice_num(BLUE_PIN); pwm_set_mask_enabled(enable_mask, true); ``` We can't change the existing function's prototype because that would break backwards-compatibility, but maybe it's worth adding a new function for this? 🤔