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.

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

Modifications for 64-bit OS #218

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Modifications for 64-bit OS #218

merged 4 commits into from
Feb 16, 2024

Conversation

TMRh20
Copy link
Member

@TMRh20 TMRh20 commented Feb 16, 2024

All I did was change unsigned long to uint32_t in examples, and now the formatting check is failing lol. I'm thinking we should use the version of clang-format that ships with RPi, which is currently 14.0.6 on my RPi5.

In any case, I'm not sure whether to make the same changes in the regular Arduino and Pico examples, because I don't think there are any 64-bit Arduinos yet.
We might want to change it for continuity, so they all read the same, but that would be the only reason I think.

- Change unsigned long to uint32_t in examples
@TMRh20 TMRh20 requested a review from 2bndy5 February 16, 2024 10:13
@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

I'm thinking we should use the version of clang-format that ships with RPi, which is currently 14.0.6 on my RPi5

I have a PR in the org repo to update to v17 v16. See nRF24/.github#11. But I see clang-format v16 is the latest available on my RPi4:

brendan@rpi4-64bit:~ $ sudo apt list clang-format*
Listing... Done
clang-format-13/stable 1:13.0.1-11+b2 arm64
clang-format-13/stable 1:13.0.1-11+b2 armhf
clang-format-14/stable 1:14.0.6-12 arm64
clang-format-14/stable 1:14.0.6-12 armhf
clang-format-15/stable 1:15.0.6-4+b1 arm64
clang-format-15/stable 1:15.0.6-4+b1 armhf
clang-format-16/stable 1:16.0.6-15~deb12u1 arm64
clang-format-16/stable 1:16.0.6-15~deb12u1 armhf
clang-format/stable 1:14.0-55.7~deb12u1 arm64
clang-format/stable 1:14.0-55.7~deb12u1 armhf

@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

I'm not sure whether to make the same changes in the regular Arduino and Pico examples

Whatever allocates a smaller footprint is preferable I think.

@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

It is odd (& annoying) that unchanged files passed the checks in master but are failing on this branch (RF24Network_config.h and RF24Network.cpp). I can limit the check-formatting step to only analyze changed lines...

Copy link
Member

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

I'm going to have to update .clang-format config file in a separate PR (on all repos). So, I'm good with this as it is.

@TMRh20
Copy link
Member Author

TMRh20 commented Feb 16, 2024

RPi3 & RPi2: RPi OS 32-bit

clang-format-11/stable 1:11.1.0-6+rpi2 armhf
clang-format-13/stable 1:13.0.1-11+rpi1+b1 armhf
clang-format-14/stable,now 1:14.0.6-12+rpi1 armhf [installed,automatic]
clang-format-15/stable 1:15.0.6-4+rpi1+b1 armhf
clang-format/stable,now 1:14.0-55.7~deb12u1 armhf [installed]

RPi5: RPi OS 64-bit

clang-format-13/stable 1:13.0.1-11+b2 arm64
clang-format-13/stable 1:13.0.1-11+b2 armhf
clang-format-14/stable,now 1:14.0.6-12 arm64 [installed,automatic]
clang-format-14/stable 1:14.0.6-12 armhf
clang-format-15/stable 1:15.0.6-4+b1 arm64
clang-format-15/stable 1:15.0.6-4+b1 armhf
clang-format-16/stable 1:16.0.6-15~deb12u1 arm64
clang-format-16/stable 1:16.0.6-15~deb12u1 armhf
clang-format/stable,now 1:14.0-55.7~deb12u1 arm64 [installed]
clang-format/stable 1:14.0-55.7~deb12u1 armhf

So v16 is available on newer RPis, but the default install across versions is v14.0.6 it looks like.
As long as I can format stuff I'm good lol.

@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

Ok, I'll adjust for v14.

@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

BTW, you can use winget install LLVM --version xx.y.z to install clang-format (and other LLVM tools) on windows 10+. 😉 I rarely push commits directly from my RPi.

@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

Is this something we should also push to 1.x branch?

@TMRh20
Copy link
Member Author

TMRh20 commented Feb 16, 2024

Hmm, I forgot about the 1.x branch already. I'll do that one right before merging these changes I guess.

@2bndy5
Copy link
Member

2bndy5 commented Feb 16, 2024

It is odd (& annoying) that unchanged files passed the checks in master but are failing on this branch (RF24Network_config.h and RF24Network.cpp). I can limit the check-formatting step to only analyze changed lines...

We might have been using v12.0.0 before, but the CI seems to be using v12.0.1 now. I don't know if that was the actual problem though.

TMRh20 added a commit that referenced this pull request Feb 16, 2024
- Modify examples files for 64-bit OS #218
@TMRh20 TMRh20 merged commit 179e0c0 into master Feb 16, 2024
22 checks passed
@TMRh20 TMRh20 deleted the 64bit_OS_Modifications branch February 16, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants