Makefile.include: fix override headers from application

[leandrolanzieri]

Contribution description

As our drivers guide states:

The idea is that this file can be overridden by an application or a board, by simply putting a file with the same name in the application's or the board's include folders, while RIOT's build system takes care of preferring those files instead of the default params file.

For some reason this doesn't seem to be working. I'm not sure whether there was some uncaught regression at some point. Currently, when one places a <driver>_params.h this doesn't override the one define by RIOT, and is simply ignored.

This PR fixes the issue by including the APPDIR in the considered include paths, so that it has priority.

Testing procedure

• On some driver test application, attempt to override default parameters by defining a <driver>_params.h file in your app directory.

Issues/PRs references

None

[riot-ci]

Murdock results

❌ FAILED

3120b0c tests/net/ieee802154_hal: rename common.h

Success	Failures	Total	Runtime
1857	0	9329	03m:35s

Artifacts

[leandrolanzieri]

Hmm it's failing on a test that tries to override the irq.h file. Seems like an existing issue.

[leandrolanzieri]

I included a commit that seems to fix this (at least this failing case) locally.

[maribu]

ACK. Thanks for fixing this! ❤️

[maribu]

Thanks for fixing this! I have falling into this pitfall numerous times when I needed to quickly test something, but never really traced it down but instead did a quick-and-dirty hack of the system header instead.

I'm glad that next time I need to quickly test something, I won't fall into that pitfall again ❤️

[leandrolanzieri]

I wonder if the irq.h header introduced in #17066 ever had an effect. Currently seems to be breaking for some builds, and even some prototypes are not correct. Was the idea to prevent inline implementations from some platforms? Do you know @maribu ?

[maribu]

I'm almost certain that header was never included before, so it can be safely removed.

I think the idea was to have the signatures of the IRQ functions available, so that setting up the mockup wrapper functions is possible. Since the mock wrapper functions replace the actual calls, having a working IRQ header included was not needed. They just would need to match the API. (For the mock ups it would not matter if they are static inline or real symbol generating functions.)

But as the test worked just fine with wrapping the IRQ functions using the real signatures (including static inline for most platforms), I'd personally just drop the irq.h from the test without giving it a second thought.

[leandrolanzieri]

After some fiddling around, it turns out it's not so straight forward. Removing the header seems to uncover other issues:

• Currently there's no way to override IRQ_API_INLINED
• Most of the platforms include irq_arch.h directly instead of irq.h

IIUC the "official" interface is to always go through irq.h, which in turn includes irq_arch.h on platforms that set IRQ_API_INLINED. We'd also need to change that.

The test would then define IRQ_API_INLINED to 0, which would allow to provide mock implementations from the application.

[maribu]

Ah, OK. So irq.h was included, but only from the tests C source (as #include "irq.h" would also search the current folder).

Now, the header is overriding the systems irq.h.

Maybe just copy-paste the contents of irq.h in the test source file and drop the header then?

[leandrolanzieri]

Maybe just copy-paste the contents of irq.h in the test source file and drop the header then?

Now this is implemented on 78413fe. It feels a bit off, but the alternative (I believe) would be a whole restructuring of how irq.h and irq_arch.h are included throughout multiple CPUs, just to mock this.

[mguetschow]

Looks good to me, thanks! Do you know what was failing in the last Murdock build? Any chance we can get this in before hard-feature freeze next week?