Skip to content

cpu/atmega: cleanup dependencies handling#12489

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
aabadie:pr/cpu/atmega_cleanup_dep
Oct 18, 2019
Merged

cpu/atmega: cleanup dependencies handling#12489
benpicco merged 1 commit intoRIOT-OS:masterfrom
aabadie:pr/cpu/atmega_cleanup_dep

Conversation

@aabadie
Copy link
Contributor

@aabadie aabadie commented Oct 18, 2019 •
edited
Loading

Contribution description

While looking at the changes in #12411, I noticed duplicated inclusions of modules in different files.
Example:

# expand atmega_pcint for atmega128rfa1
ifneq (,$(filter atmega_pcint,$(USEMODULE)))
  USEMODULE += atmega_pcint1
endif

was used in Makefile.include and Makefile.dep. Comparing with other atmega cpu, the problem was there all over the place.

This PR is trying to clean this up a little:

  • Create a Makefile.dep with the handling of additional pcint modules
  • Remove duplicates addition of pcint modules from Makefile.include
  • Move USEMODULE += atmega_common to the common atmega Makefile.dep.

Testing procedure

A green Murdock should be ok but maybe more functional tests could also be done.
Maybe also compare the modules included in a build with master (not sure how to do that).

Issues/PRs references

None

@aabadie aabadie added Area: build system Area: Build system 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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: cpu Area: CPU/MCU ports labels Oct 18, 2019
@aabadie aabadie requested a review from benpicco October 18, 2019 06:35
@benpicco
Copy link
Contributor

benpicco commented Oct 18, 2019

I think the easiest check to see if you did catch all duplicates is grep -r "USEMODULE += atmega_pcint" cpu.

before:

cpu/atmega1284p/Makefile.include:  USEMODULE += atmega_pcint1 atmega_pcint2 atmega_pcint3
cpu/atmega1284p/Makefile.dep:  USEMODULE += atmega_pcint1 atmega_pcint2 atmega_pcint3
cpu/atmega128rfa1/Makefile.include:  USEMODULE += atmega_pcint1
cpu/atmega128rfa1/Makefile.dep:  USEMODULE += atmega_pcint1
cpu/atmega2560/Makefile.include:  USEMODULE += atmega_pcint1 atmega_pcint2
cpu/atmega2560/Makefile.dep:  USEMODULE += atmega_pcint1 atmega_pcint2
cpu/atmega256rfr2/Makefile.include:  USEMODULE += atmega_pcint1
cpu/atmega256rfr2/Makefile.dep:  USEMODULE += atmega_pcint1
cpu/atmega_common/Makefile.include:  USEMODULE += atmega_pcint0
cpu/atmega_common/Makefile.dep:  USEMODULE += atmega_pcint0
cpu/atmega328p/Makefile.include:  USEMODULE += atmega_pcint1 atmega_pcint2
cpu/atmega1281/Makefile.include:  USEMODULE += atmega_pcint1 atmega_pcint2

after:

cpu/atmega1284p/Makefile.dep:  USEMODULE += atmega_pcint1 atmega_pcint2 atmega_pcint3
cpu/atmega128rfa1/Makefile.dep:  USEMODULE += atmega_pcint1
cpu/atmega2560/Makefile.dep:  USEMODULE += atmega_pcint1 atmega_pcint2
cpu/atmega256rfr2/Makefile.dep:  USEMODULE += atmega_pcint1
cpu/atmega_common/Makefile.include:  USEMODULE += atmega_pcint0
cpu/atmega_common/Makefile.dep:  USEMODULE += atmega_pcint0
cpu/atmega328p/Makefile.dep:  USEMODULE += atmega_pcint1 atmega_pcint2
cpu/atmega1281/Makefile.dep:  USEMODULE += atmega_pcint1 atmega_pcint2

So looks like you missed on in cpu/atmega_common.

@aabadie
Copy link
Contributor Author

aabadie commented Oct 18, 2019

So looks like you missed on in cpu/atmega_common

Indeed!

@aabadie aabadie force-pushed the pr/cpu/atmega_cleanup_dep branch from ddb737d to d65375d Compare October 18, 2019 09:36
@benpicco
Copy link
Contributor

benpicco commented Oct 18, 2019

Just squash already.

@aabadie aabadie force-pushed the pr/cpu/atmega_cleanup_dep branch from d65375d to 629003d Compare October 18, 2019 10:40
@aabadie
Copy link
Contributor Author

aabadie commented Oct 18, 2019

Are we ok here @benpicco ?

benpicco
benpicco approved these changes Oct 18, 2019
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

grep confirms that all duplicates were removed, but not more.

@benpicco benpicco merged commit e9d6a61 into RIOT-OS:master Oct 18, 2019
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
@aabadie aabadie deleted the pr/cpu/atmega_cleanup_dep branch March 5, 2020 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

Assignees

No one assigned

Labels

Area: build system Area: Build system 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 Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

3 participants

@aabadie @benpicco @fjmolinas