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

CMake for Linux #176

Merged
merged 95 commits into from
Nov 14, 2021
Merged

CMake for Linux #176

merged 95 commits into from
Nov 14, 2021

Conversation

2bndy5
Copy link
Member

@2bndy5 2bndy5 commented Jun 12, 2021

The usage of CMake for this library follows suite with the same implementation in the RF24 repo (except the RF24_DRIVER option). Any user-adjustable defines in RF24Network_config.h can be changed from CMake CLI options.

New build-linux workflow that

  • cross-compiles & packages the library targeting armhf & arm64 architectures
    • packages targeting armhf & arm64 architectures will be automatically uploaded to future releases' assets
  • tests the python wrapper installation (not cross-compiled though)
  • tests the library examples (not the python examples though)

A new badge in README now reports the latest status of the Linux build CI workflow


  • Additional improvements to the CI workflow scripts that limit triggers to only when relevant files are modified.
  • python wrapper now includes metadata recognized by pypi.org (specifically the license agreement)
  • fixed docs about when RF24_LINUX is defined and forced (via DOXYGEN_FORCED - only defined when doxygen is run) Arduino-only members to be rendered as well ass linux-only members. Technically, the docs were already written for this, but the #ifdef statements confused doxygen (thus doxygen output incorrect CLI warnings).
  • introduced 2 new macro definitions to replace a couple sentinel vales used repeatedly throughout the source code. Also replaced some calls to write() that used an existing macro defined value instead of passing the value using the macro defined name.
  • renamed write()'s directTo parameter to sendType for clarity.
  • found & fixed a malformed relative hyperlink in addressing.md
  • unified the debug output functionality to depend on macros defined in RF24_config.h. This means that printf_P, PSTR, and PRIPSTR are used inside the IF_SERIAL_DEBUG*() calls. This change should also fix printing the return c-string from RF24NetworkHeader::toString() (by using PRIPSTR) for some debugging outputs on various platforms.
  • The maxlen parameter to read() and peek(header) are now optional by using the MAX_PAYLOAD_SIZE as a default value. The mentioned functions will only handle the maximum message length if not specified or a value greater than the message's length is passed. This change is also reflected in the python wrapper.
  • The level parameter to the multicast() function is now optional. Additionally, if a value greater than 4 is passed to the level parameter, then the node's currently configured multicast_level (private member) is used as a default. This change is also reflected in the python wrapper.
  • improved outgoing fragmentation by avoiding unnecessary SPI transactions amidst transmitting fragments.
  • reduced computation in logicalToPhysicalAddress() (private member) where a function was unnecessarily called twice, but no longer.
  • added missing public members to the python wrapper:
    • the newer/preferred begin(node_address); the old/deprecated begin(channel, node_address) is still present.
    • multicast_level
    • multicast()
    • multicastRelay
    • routeTimeout
    • parent
    • networkFlags
    • is_address_valid()
  • removed unused FLAG_BYPASS_HOLDS & FLAG_HOLD_INCOMING as they are not used in any RF24* lib (actually relics from early development).
  • all examples no longer use the deprecated begin(channel, address) function. Instead they use RF24::setChannel() & RF24Network::begin(addr)

@2bndy5 2bndy5 self-assigned this Jun 12, 2021
@2bndy5 2bndy5 linked an issue Jun 19, 2021 that may be closed by this pull request
@2bndy5 2bndy5 linked an issue Aug 24, 2021 that may be closed by this pull request
@2bndy5 2bndy5 marked this pull request as ready for review October 24, 2021 13:43
@TMRh20
Copy link
Member

TMRh20 commented Nov 14, 2021

Is this ready to go? The only thing I see missing is instructions on how to build using cmake in the RF24 docs.

@see Also, [Linux Installation](http://nRF24.github.io/RF24/md_docs_linux_install.html) and [General Linux/RPi configuration and setup](http://nRF24.github.io/RF24/md_docs_rpi_general.html) documentation
@see
[RF24 Library docs](http://nRF24.github.io/RF24/) for general RF24 configuration and setup.
- [Linux Installation](http://nRF24.github.io/RF24/md_docs_linux_install.html) and [General Linux/RPi configuration and setup](http://nRF24.github.io/RF24/md_docs_rpi_general.html) documentation
Copy link
Member Author

Choose a reason for hiding this comment

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

These links will lead users to the build system they prefer. RF24/md_docs_linux_install.html has an advisory at the top of the page now. Its not explicit in this lib's docs, but then again, it never really was.

@2bndy5
Copy link
Member Author

2bndy5 commented Nov 14, 2021

yeah its ready to go unless you have some requests

@TMRh20
Copy link
Member

TMRh20 commented Nov 14, 2021

From your notes

The level parameter to the multicast() function is now optional. Additionally, if a value greater than 3 is passed to the level parameter, then the node's currently configured multicast_level (private member) is used as a default. This change is also reflected in the python wrapper.

I believe the max should be 4 since there are 5 multicast levels

RF24Network.cpp Outdated Show resolved Hide resolved
@2bndy5
Copy link
Member Author

2bndy5 commented Nov 14, 2021

You seem to be on a cleaning-streak. If you want to continue with the other Cmake-Linux PRs, then they have to merged to master in the OSI layer's order. Meaning, merge this and I'll mark RF24Mesh PR "ready for review".

@TMRh20 TMRh20 merged commit 02c46f8 into master Nov 14, 2021
@2bndy5
Copy link
Member Author

2bndy5 commented Nov 14, 2021

After looking at the docs_build workflow artifacts, it seems that only sphinx is having trouble rendering the topology image.
image
Meanwhile this page renders fine in doxygen (inspected via workflow artifacts) and github. This may be a bug with the sphinx extension breathe... I'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment