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

cpu/esp_common: esp-wifi: drop assert(val) #19854

Merged
merged 1 commit into from Aug 3, 2023
Merged

cpu/esp_common: esp-wifi: drop assert(val) #19854

merged 1 commit into from Aug 3, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Aug 1, 2023

Contribution description

The assert() in _esp_wifi_get is excessive and wrong for NETOPT_IS_WIRED where val == NULL is valid.

Testing procedure

Running applications that query the NETOPT_IS_WIRED option (e.g. gnrc_ipv6_static_addr) should now work without triggering the assertion.

Issues/PRs references

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

@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 labels Aug 1, 2023
@github-actions github-actions bot added Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Aug 1, 2023
@benpicco benpicco requested a review from maribu August 1, 2023 21:11
@riot-ci
Copy link

riot-ci commented Aug 1, 2023

Murdock results

✔️ PASSED

94771f9 cpu/esp_common: esp-wifi: drop assert(val)

Success Failures Total Runtime
7907 0 7907 12m:18s

Artifacts

@gschorcht
Copy link
Contributor

borse merge

@gschorcht
Copy link
Contributor

Why is merging blocked?

@benpicco
Copy link
Contributor Author

benpicco commented Aug 2, 2023

borse merge

😉

bors merge

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>
@gschorcht
Copy link
Contributor

borse merge

😉

bors merge

🙈

@RIOT-OS RIOT-OS deleted a comment from bors bot Aug 2, 2023
@gschorcht
Copy link
Contributor

borse merge

😉
bors merge

🙈

Auto correction on mobile

@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 #19854
c5b3194 
19854: cpu/esp_common: esp-wifi: drop assert(val) 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 2, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 3, 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 63062aa into RIOT-OS:master Aug 3, 2023
29 checks passed
@benpicco benpicco deleted the cpu/esp_common/esp-wifi-assert branch August 3, 2023 08:31
@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

@gschorcht gschorcht gschorcht approved these changes

@maribu maribu Awaiting requested review from maribu

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports 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 Platform: ESP Platform: This PR/issue effects ESP-based platforms 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.

None yet
4 participants
@benpicco @riot-ci @gschorcht @MrKevinWeiss