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

[LoRaWAN] Accept const uint8_t* on public API #1302

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

Kabbah
Copy link
Contributor

@Kabbah Kabbah commented Oct 31, 2024

The goal of this PR is allowing the caller to pass const uint8_t* parameters to the public methods of LoRaWANNode.

Some LoRaWANNode methods, such as beginOTAA, take buffers as parameters (in this case, the AppKey and NwkKey) with type uint8_t*. However, in many cases, the buffers are not modified and could be specified as const. Not using const makes the caller unable to pass a const uint8_t APP_KEY[16], for example, resulting in a compilation error such as:

error: invalid conversion from 'const uint8_t*' {aka 'const unsigned char*'} to 'uint8_t*' {aka 'unsigned char*'}

In this PR, I focused just on public methods, but there may be other opportunities to use pointers to const elsewhere in the file. Unfortunately, the change leaked into Utils.cpp, as RADIOLIB_DEBUG_PROTOCOL_HEXDUMP calls rlb_hexdump, which also takes a non-const parameter despite not modifying it. It should not change behavior, but if you'd rather avoid changing it, I can undo the const change in setBufferNonces.

@jgromes
Copy link
Owner

jgromes commented Nov 1, 2024

Looks good to me - @StevenCellist maybe this will require some changes in https://github.com/radiolib-org/radiolib-persistence?

@StevenCellist
Copy link
Collaborator

Probably, I'm not an expert at the const stuff. I can test for the ESP32 tomorrow, but I dread the moment I have to take out my ESP8266 as I lack one GPIO pin. Overall, looks fine to me.

@Kabbah
Copy link
Contributor Author

Kabbah commented Nov 1, 2024

I believe it should have no impact on existing user code, radiolib-persistence included. Using uint8_t appKey[] = { /* ... */ } and such is still allowed, because uint8_t* is promoted to const uint8_t*.

Adding const to the library is like saying "I promise not to modify this buffer", and that is already the case.

@jgromes jgromes merged commit a608075 into jgromes:master Nov 1, 2024
28 checks passed
@jgromes
Copy link
Owner

jgromes commented Nov 1, 2024

@Kabbah thanks for the additional context - merged, thank you for the contribution!

@Kabbah Kabbah deleted the lorawan-const-pointers branch November 1, 2024 18:42
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.

3 participants