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: make possible to send empty frames #72600
lorawan: make possible to send empty frames #72600
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with that change, only got a small suggestion for improvement.
I'd leave the NULL
check though, unless you have double-checked that the LoRaWAN will definitely fail gracefully even for data == NULL && len > 0
.
subsys/lorawan/lorawan.c
Outdated
@@ -595,7 +595,7 @@ int lorawan_send(uint8_t port, uint8_t *data, uint8_t len, | |||
int ret = 0; | |||
bool empty_frame = false; | |||
|
|||
if (data == NULL) { | |||
if (len && data == NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to make the len
check more explicit and swap the conditions, so that the second condition doesn't have to be evaluated in most cases:
if (len && data == NULL) { | |
if (data == NULL && len > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, I have just updated my PR.
The |
Empty frames are allowed by the LoRaWAN protocol and are actually useful to open new RX slots or flush MAC commands in stack. Therefore allow the data pointer to be NULL if len is 0. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
e76ee0b
to
1a5e412
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QoL change, you can already send a 0 length packet by proving a non-NULL pointer with a length of 0.
Empty frames are allowed by the LoRaWAN protocol and are actually useful to open new RX slots or flush MAC commands in stack. Therefore allow the data pointer to be
NULL
iflen
is 0.Note that I am not even sure we want to have this check. It seems in other subsystems, it is the responsibility of the caller to ensure the data pointer is not NULL.