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

Jump to bottom

gnrc_static: fix static PID assignment #19855

Merged
merged 1 commit into from
Aug 7, 2023
Merged

gnrc_static: fix static PID assignment #19855

merged 1 commit into from
Aug 7, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Aug 1, 2023 •
edited
Loading

Contribution description

Both CONFIG_GNRC_IPV6_STATIC_ADDR_UPSTREAM and CONFIG_GNRC_IPV6_STATIC_ADDR_DOWNSTREAM get set to 0 if they are not defined, so we don't need the IS_ACTIVE() - in fact, using IS_ACTIVE() leads to wrong results here.

Testing procedure

Build an application with

CFLAGS += -DCONFIG_GNRC_IPV6_STATIC_ADDR_UPSTREAM=8       # netif-esp-wifi
CFLAGS += -DCONFIG_GNRC_IPV6_STATIC_ADDR_DOWNSTREAM=9     # slipdev

Turns out the IS_ACTIVE(DCONFIG_GNRC_IPV6_STATIC_ADDR_UPSTREAM) would evaluate to false.

Issues/PRs references

https://forum.riot-os.org/t/border-router-ota-using-wifi/3895/24

@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Aug 1, 2023
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: network Area: Networking and removed Area: network Area: Networking Area: sys Area: System labels Aug 1, 2023
@riot-ci
Copy link

riot-ci commented Aug 1, 2023

Murdock results

✔️ PASSED

ae28f11 gnrc_static: fix static PID assignment

Success Failures Total Runtime
7907 0 7907 11m:09s

Artifacts

@benpicco benpicco requested a review from maribu August 1, 2023 21:53
@maribu
Copy link
Member

maribu commented Aug 2, 2023

Maybe we should also align the semantics of IS_ACTIVE() with standard C that anything non-zero is considered true. Otherwise this ia pretty much a foot gun.

@benpicco
Copy link
Contributor Author

benpicco commented Aug 2, 2023

Certainly!
But unnecessary use of IS_ACTIVE() also makes the code less readable, so it's always good if we can avoid it.

bors merge

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors @benpicco
Merge #19855
7224a31 
19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
bors bot added a commit that referenced this pull request Aug 2, 2023
@bors @benpicco
Merge #19854 #19855
363b03a 
19854: cpu/esp_common: esp-wifi: drop assert(val) r=benpicco a=benpicco



19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
@RIOT-OS RIOT-OS deleted a comment from bors bot Aug 2, 2023
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors @benpicco
Merge #19855
a89fbd0 
19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

Build failed:

@benpicco
Copy link
Contributor Author

benpicco commented Aug 7, 2023

bors merge

bors bot added a commit that referenced this pull request Aug 7, 2023
@bors @gschorcht @benpicco
Merge #19824 #19855
0462a8c 
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
@bors
Copy link
Contributor

bors bot commented Aug 7, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Aug 7, 2023
@bors @benpicco
Merge #19855
5bf9a30 
19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
@benpicco
Copy link
Contributor Author

benpicco commented Aug 7, 2023

bors cancel

@bors
Copy link
Contributor

bors bot commented Aug 7, 2023

Canceled.

@benpicco
Copy link
Contributor Author

benpicco commented Aug 7, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 7, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit d512ff6 into RIOT-OS:master Aug 7, 2023
@benpicco benpicco deleted the gnrc_ipv6_static_addr-fix_pid branch August 7, 2023 20:12
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maribu maribu maribu approved these changes

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

@cgundogan cgundogan Awaiting requested review from cgundogan

Assignees
No one assigned
Labels
Area: network Area: Networking 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
Milestone
Release 2023.10
Development

Successfully merging this pull request may close these issues.
4 participants
@benpicco @riot-ci @maribu @MrKevinWeiss