Repository navigation
zephyr-cp: add pwmio for nordic boards - #11525
Merged
Merged
Conversation
iobroker allocates PWM channels in period groups (an nRF instance's four outputs share COUNTERTOP): `iobroker_pwm_channel_allocate()` adjusts the frequency to a period the hardware produces (truncating on nRF) and joins a running group with the same period, or takes a free instance. Joining or leaving configures only that output's PSEL, so running channels are not disturbed. `variable_frequency` gets an exclusive group, and `iobroker_pwm_period_cycles()` adjusts its new frequency. common-hal/pwmio uses only Zephyr's PWM API, so other SoCs need just an iobroker implementation (an RP2 slice or an STM32 timer is a group). Enabled where `CONFIG_PWM_NRFX` is set, like neopixel_write. Tested with a logic analyzer on the Feather nRF52840 and nRF54LM20 DK, and on the nRF54L15 DK. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the pwm_emul module: an adafruit,pwm-emul controller that records what `pwm_set_cycles()` receives and, with Perfetto tracing, emits it on per-channel `pwm.NN.period` and `pwm.NN.pulse` counter tracks. Channel N stands for GPIO N. iobroker's emul/iobroker_pwm.c allocates its channels with a pool of period groups that any channel can join, which models only the pwmio rules every port shares: equal frequencies share a group, a variable frequency gets its own, and with no usable group left allocation fails. native_sim gets one 32-channel controller with two groups at 16 MHz, and zephyr2cp enables pwmio when `CONFIG_PWM_ADAFRUIT_EMUL` is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Check the period and pulse traced for each pin and pwmio's behavior: frequency and duty conversion, the reported frequency, frequency changes, invalid frequencies, pin claims, group sharing and exhaustion, and variable-frequency groups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tannewt
requested changes
Oct 8, 2026
tannewt
left a comment
Member
There was a problem hiding this comment.
Thanks for this! I like the perfetto output used in tests. I added a couple comments about the IOBroker API.
1acf65e removed its only call and every port's definition but left the declaration in shared-bindings; drop it and the zephyr-cp stub. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
iobroker now uses the requested frequency only to decide which channels share a group, and returns just the device and channel. common-hal/pwmio turns the frequency into a period from `pwm_get_cycles_per_sec()` (truncating, as before) and passes it to `pwm_set_cycles()`. `iobroker_pwm_period_cycles()` and the `period_cycles_out` parameter are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Remove the public whole-instance `iobroker_pwm_allocate()`. A PWM allocation is always a channel, identified by its device and channel number, like the GPIO, ADC and DAC allocations. A caller that needs a group to itself asks for an exclusive channel: pwmio for a variable frequency, and neopixel_write, which programs a whole nRF instance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`pwm_nrfx` skips the period check at 0 % and 100 % duty and never checks the minimum COUNTERTOP, so out-of-range frequencies can be accepted. Say so in the iobroker README and pwmio comments; validation will be handled separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- PWMOut frequency setter: say what shared-bindings validates, and remove the frequency == 0 check that validation makes unreachable. - iobroker.h: say iobroker_pwm_channel_allocate() doesn't set the frequency, list what a group is on each SoC, and split the comment into paragraphs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Ready for re-review. Whole PWM allocator removed, as described above. Did not attempt to ameliorate lousy arg validation in SOC-specific |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude wrote most of the code, with @dhalbert steering, reviewing and testing.
pwmiofor the zephyr-cp port on Nordic. IOBroker already had some support for PWM due to neopixel_write.iobroker_pwm_channel_allocate(pin, frequency, exclusive, ...)returns a device and channel. It doesn't actually set the frequency. It uses the frequency only to choose a group, either a running group requested at the same frequency, or else a free instance.exclusiverequest gets a group of its own. Avariable_frequency=TruePWMOutsetsexclusive, so it can change its period without affecting other channels.iobroker_pwm_channel_release()returns the channel. The group's last channel releases the instance.common-hal/pwmioturns the frequency into a period itself, withpwm_get_cycles_per_sec(). The period is truncated, as inports/nordic, so the actual frequency is the same or slightly higher.neopixel_writenow allocates an exclusive channel, which on nRF is a whole instance. There is no separate whole-instance allocator.Not in this PR
pwm_nrfxdoesn't reject every period it can't produce: it accepts 1–3 Hz, and about 5.3–8 MHz. This will be fixed separately, by a validator or by patching the drivers.Future
pwmioimplementationscommon-hal/pwmiouses only Zephyr's PWM API. Another SoC needs just an iobroker implementation; an RP2 slice or an STM32 timer would be a group.Tests
pwm_emulmodule for tests: an emulated PWM controller,adafruit,pwm-emul, that traces each pin's period and pulse as Perfetto counter tracks. native_sim gets one channel per pin and two period groups that any channel can join. This models only thepwmiorules every port shares, not any SoC's layout.tests/test_pwmio.py: frequency and duty conversion, the reported frequency, frequency changes, invalid frequencies, pin claims, group sharing and exhaustion, and variable-frequency groups.Testing
neopixel_writeon an RGBW ring and the status NeoPixel, on the PWM path and on the bit-bang fallback with every instance held bypwmio.ValueError.ValueError.test_pwmio.pyon native_sim, native_sim_asan and native_sim_lfs.