Skip to content

cpu/atmega_common/periph/gpio: Clean up PCINT support#12933

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
maribu:atmega_pcint_cleanup
Feb 6, 2020
Merged

cpu/atmega_common/periph/gpio: Clean up PCINT support#12933
benpicco merged 2 commits intoRIOT-OS:masterfrom
maribu:atmega_pcint_cleanup

Conversation

@maribu
Copy link
Member

@maribu maribu commented Dec 12, 2019

Contribution description

  • Fixed a minor bug in the PCINT implementation (first commit)
  • Some cleanup in the PCINT implementation (second commit):
    • Using an enum instead of the preprocessor is a bit more readable

Testing procedure

gpio_init_int() should still work as before on ATmega boards. (Both with USEMODULE += atmega_pcint and without.) PR #11923 can be useful to test this.

Issues/PRs references

None

The bank index and the pin number are not necessarily identical. For all
PCINT banks except for bank 3 bank_idx was used therefore. It was likely
just forgotten to update that for bank 3 as well.
- Using a enum instead of _COUNTER is easier to read
    - _COUNTER is also a reserved name; so better not use it to avoid issues
- Split out the pcint code into a static inline function for increased
  readability
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: cpu Area: CPU/MCU ports labels Dec 12, 2019
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 15, 2019
@maribu
Copy link
Member Author

maribu commented Feb 6, 2020

Ping?

Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

tested ACK

@smlng smlng added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Feb 6, 2020
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 6, 2020
@benpicco benpicco merged commit 24fb7a9 into RIOT-OS:master Feb 6, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 20, 2020
@maribu maribu deleted the atmega_pcint_cleanup branch September 15, 2020 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants