Skip to content

drivers/at24mac: fix dependency#15449

Closed
benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
benpicco:drivers/at24mac-dep
Closed

drivers/at24mac: fix dependency#15449
benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
benpicco:drivers/at24mac-dep

Conversation

@benpicco
Copy link
Contributor

@benpicco benpicco commented Nov 16, 2020

Contribution description

The at24mac driver does not depend on at24cxxx, EEPROM and ID functionality are separate.

Testing procedure

tests/driver_at24mac still works

2020-11-16 12:15:07,922 # main(): This is RIOT! (Version: 2021.01-devel-903-g96afd-drivers/at24mac-dep)
2020-11-16 12:15:07,926 # EUI-48: fc c2 3d 0d 2d 1f
2020-11-16 12:15:07,932 # ID-128: 0a 70 08 00 64 10 04 81 1c cd a0 00 a0 00 00 00
2020-11-16 12:15:07,933 # [SUCCESS]

Issues/PRs references

The at24mac driver does not depend on at24cxxx, EEPROM and ID
functionality are separate.
@benpicco benpicco added Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Nov 16, 2020
@aabadie
Copy link
Contributor

aabadie commented Nov 16, 2020

accidentally introduced by #14386

It was already like this when #14386 was opened.

@benpicco
Copy link
Contributor Author

Huh this is odd.
I now remember that I added this dependency because I thought the linker would just throw out all at24cxxx_ functions if they are not used, so only having to include one module is more convenient.

Yet this saves 988 bytes of ROM on tests/driver_at24mac compared to master.

@benpicco
Copy link
Contributor Author

benpicco commented Nov 16, 2020

ah this is because at24cxxx pulls in xtimer and that will cause changes to core functionality even if xtimer is not called by the application.

So yea, I don't know if this makes a difference in practice.
The dependency is only for convenience, should we still have it?

@benpicco
Copy link
Contributor Author

at24c% pulls in at24cxxx so it makes sense that the user should never have to manually include at24cxxx.
In that sense having the dependency for at24mac is consistent - and any application that uses MAC addresses will also use xtimer.

I think this can be closed.

@bergzand bergzand 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 Nov 16, 2020
@benpicco benpicco closed this Nov 16, 2020
@benpicco benpicco deleted the drivers/at24mac-dep branch November 16, 2020 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants