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.

Sign up for GitHub

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

examples/gnrc_border_router: pass variables to docker #20618

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+10 −0
Open

examples/gnrc_border_router: pass variables to docker #20618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de0036f
build system: pass WIFI_SSID and WIFI_PASS to docker
maribu Apr 24, 2024
276ad18
examples/gnrc_border_router: pass UPLINK etc to docker
maribu Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions examples/gnrc_border_router/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ endif
# the variable to empty
SHOULD_RUN_KCONFIG ?=

# Pass through application configuration to the docker container, if
# BUILD_IN_DOCKER=1 is used:
DOCKER_ENV_VARS += IPV6_ADDR
DOCKER_ENV_VARS += IPV6_DEFAULT_ROUTER
DOCKER_ENV_VARS += IPV6_PREFIX IPV6_DEFAULT_ROUTER
DOCKER_ENV_VARS += PREFIX_CONF
DOCKER_ENV_VARS += UPLINK

include $(RIOTBASE)/Makefile.include

# Compile-time configuration for DHCPv6 client (needs to come after
Expand Down
2 changes: 2 additions & 0 deletions makefiles/docker.inc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export DOCKER_ENV_VARS += \
TOOLCHAIN \
UNDEF \
WERROR \
WIFI_PASS \
WIFI_SSID \
Comment on lines +73 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those defined and documented anywhere in a generic way? grepping through the RIOT repo only shows them being mentioned in esp-common and atwinc15x0 driver documentation, where they are expected to be passed within CFLAGS anyway.

So instead of adding them here, I would use the same approach as for the other Makefile variables just for examples/gnrc_border_router.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are actually not used by the example, but used by the only two WIFI drivers (ESP* and ATWINC15x0) we have.

They are documented separately in both drivers.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I follow. The example defines the environment variables in https://github.com/RIOT-OS/RIOT/blob/master/examples/gnrc_border_router/Makefile#L19-L20 and uses them in https://github.com/RIOT-OS/RIOT/blob/master/examples/gnrc_border_router/Makefile.wifi.conf to add them to CFLAGS.

The documentation of both ESP* and the ATWINC15x0 explicitly adds them to CFLAGS as well (e.g., https://github.com/RIOT-OS/RIOT/blob/master/cpu/esp32/doc.txt#L1639). The only other example I could find that uses these environment variables is examples/paho-mqtt which also explicitly adds them to CFLAGS later on: https://github.com/RIOT-OS/RIOT/blob/master/examples/paho-mqtt/Makefile#L62-L63.

So my point was that the environment variables themselves are not generically defined, but only custom to specific examples, and therefore I think those examples should add the respective DOCKER_ENV_VARS themselves.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maribu ping

Might be nice to have this in before the hard feature freeze in two weeks from now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the best would to actually move extending the CFLAGS with the wifi password to a common place and documenting it. I have the feeling that every non-testing app that includes networking and will copy-paste that snipped anyway the moment someone starts using that app for a board with WiFi connectivity anyway.

#

# List of all exported environment variables that shall be passed on to the
Expand Down
Loading