Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename dma::SingleChannel::listen_irq* to enable_irq* #648

Merged
merged 2 commits into from
Jul 16, 2023

Conversation

nilclass
Copy link
Contributor

My motivation for this change is that the first time I encountered the listen_irq* methods, the name invoked a misleading picture in my mind:
channel.listen_irq0() sounds like the channel is "listening" for the IRQ, and will do something once it triggers.

That's not the case though: calling this method will cause the channel to trigger the interrupt -- some other part of the program must do the listening.

Reasons to call it enable_irq* / disable_irq* instead:

  • The corresponding function in pico-sdk is called dma_channel_set_irqX_enabled
  • The INTEx register documentation also uses this terminology: "Interrupt Enables for IRQ X"

My motivation for this change is that the first time I encountered the
`listen_irq*` methods, the name invoked a misleading picture in my
mind: `channel.listen_irq0()` sounds like the channel is "listening"
for the IRQ, and will do something once it triggers.
That's not the case though: calling this method will cause the channel
to *trigger* the interrupt -- some other part of the program must do
the listening.

Reasons to call it `enable_irq*` / `disable_irq*` instead:
- The corresponding function in pico-sdk is called `dma_channel_set_irqX_enabled`
- The INTEx register documentation also uses this terminology: "Interrupt Enables for IRQ X"
Copy link
Member

@jannic jannic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and the rationale is convincing.

@jannic jannic merged commit c7e6cca into rp-rs:main Jul 16, 2023
8 checks passed
@nilclass nilclass deleted the rename-dma-channel-listen branch July 16, 2023 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants