Skip to content

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

Merged
bors[bot] merged 1 commit intoRIOT-OS:masterfrom
benpicco:cpu/esp_common/esp-wifi-assert
Aug 3, 2023
Merged

cpu/esp_common: esp-wifi: drop assert(val)#19854
bors[bot] merged 1 commit intoRIOT-OS:masterfrom
benpicco:cpu/esp_common/esp-wifi-assert

Conversation

@benpicco
Copy link
Copy Markdown
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 requested a review from gschorcht as a code owner August 1, 2023 21:10
@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
Copy Markdown

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
Copy Markdown
Contributor

borse merge

@gschorcht
Copy link
Copy Markdown
Contributor

Why is merging blocked?

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Aug 2, 2023

borse merge

😉

bors merge

bors bot added a commit that referenced this pull request Aug 2, 2023
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 <[email protected]>
@gschorcht
Copy link
Copy Markdown
Contributor

borse merge

😉

bors merge

🙈

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

borse merge

😉
bors merge

🙈

Auto correction on mobile

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Aug 2, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Aug 2, 2023
19854: cpu/esp_common: esp-wifi: drop assert(val) r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <[email protected]>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Aug 2, 2023

Build failed:

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Aug 2, 2023

bors merge

@bors
Copy link
Copy Markdown
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
@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

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

Development

Successfully merging this pull request may close these issues.

4 participants