Skip to content

Conversation

@linguini1
Copy link
Contributor

Summary

Some macros used in the ieee80211.h header were missing definitions when attempting to build on Arch Linux. It appears this is a system difference between Arch Linux and Ubuntu. The definitions were:

  • hweight8
  • cpu_to_le16
  • le32_to_cpu
  • BIT

Impact

This PR replaces the cpu_to_le16 and le32_to_cpu references with their equivalent definitions htole16 and le32toh from sys/endian.h. I have also included definitions of hweight8 and BIT at the top of the file if they are found to be undefined.

Now, users on Arch Linux can compile the IEEE802.11 header. This resolves issue #13704.

Testing

After implementing these changes, I built the raspberrypi-pico-w:telnet configuration and connected to my home network successfully. I was able to ping other devices on my network.

@github-actions github-actions bot added Area: Drivers Drivers issues Size: M The size of the change in this PR is medium labels Dec 22, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 22, 2024

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

Yes, this PR appears to meet the NuttX requirements, although the provided information could be more complete.

Strengths:

  • Clear Summary: The summary explains the problem, the affected code, and the solution. The reference to issue [BUG] Cannot build telnet configuration for Raspberry Pi Pico W #13704 is excellent.
  • Impact Description: The impact section addresses user impact (Arch Linux users can now compile), build impact (implicit in the fix), and references a related issue.
  • Testing Description: The testing description indicates a successful build and basic functionality test.

Weaknesses/Missing Information:

  • No "before" logs: While "after" testing is shown (implicitly through successful build/ping), "before" logs demonstrating the build failure are missing. This makes it harder to assess the impact of the change.
  • Limited Target Information: While raspberrypi-pico-w:telnet is mentioned, more details about the build host (OS, compiler version) are needed. Ideally, testing on multiple architectures is preferred.
  • Missing Impact Sections: Several impact sections are unaddressed (hardware, documentation, security, compatibility). While some might not be applicable, explicitly stating "NO" with a brief justification is better. For example: "Impact on hardware: NO (This change only affects header file definitions)."
  • BIT Macro Definition: The summary mentions adding a definition for the BIT macro. The Impact section doesn't explicitly mention it. Clarify if this is a standard definition or a custom one, and why a system header wasn't used. If a custom definition is used, justify its correctness and potential portability issues.
  • hweight8 Definition: Similar to the BIT macro, more details on the hweight8 definition are needed. Is this a custom implementation or taken from a standard library?

Recommendation:

While the PR seems promising, adding the missing information, especially the "before" logs and clarifying the macro definitions, will significantly improve its quality and make review easier. Explicitly addressing all impact sections, even if they are not applicable, will make the PR more comprehensive.

@xiaoxiang781216 xiaoxiang781216 linked an issue Dec 22, 2024 that may be closed by this pull request
1 task
…re possible, and otherwise defined macros using built-ins to allow the 802.11 header to compile on Arch Linux where cpu_to_le16, le32_to_cpu and hweight8 definitions were missing.
@xiaoxiang781216 xiaoxiang781216 merged commit 1cbb718 into apache:master Dec 23, 2024
27 checks passed
@linguini1 linguini1 deleted the ieee80211-arch-linux branch November 9, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Drivers Drivers issues Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Cannot build telnet configuration for Raspberry Pi Pico W

3 participants