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

Cooja platform: append to CONTIKI_TARGET_SOURCEFILES rather than set them #2893

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

atiselsts
Copy link
Member

Other Contiki-NG platforms add to CONTIKI_TARGET_SOURCEFILES in their own Makefile.

Cooja simply sets the variable CONTIKI_TARGET_SOURCEFILES.

This breaks user application code that sets CONTIKI_TARGET_SOURCEFILES from the application Makefiles.

@@ -67,7 +67,7 @@ COOJA_INTFS = beep.c ip.c leds-arch.c moteid.c \
COOJA_CORE = platform.c mtype.c random.c sensors.c leds.c gpio-hal-arch.c buttons.c

# (COOJA_SOURCEFILES contains additional sources set from simulator)
CONTIKI_TARGET_SOURCEFILES = \
CONTIKI_TARGET_SOURCEFILES += \
Copy link
Contributor

Choose a reason for hiding this comment

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

doc/getting-started/The-Contiki-NG-build-system.md describes this variable as:

It contains the list of C files (CONTIKI_TARGET_SOURCEFILES) that the platform adds to the Contiki-NG system.

which I interpret as "this is a system variable not meant to be modified by applications".

@atiselsts
Copy link
Member Author

We have an inconsistency and I think there are two course of action to fix that inconsistency:
a) Change how CONTIKI_TARGET_SOURCEFILES are handled in other platforms and break user applications that rely on the undocumented behavior
b) Change the Cooja platform and the documentation
I made the PR so my opinion on this tradeoff should be pretty clear. Porting an app to a Cooja, or to a new version of Contiki-NG should not require in-depth Makefile changes from the user IMO.

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.

3 participants