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

Stm32duino changes #58

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

matthijskooijman
Copy link

While developing the STM32LoRaWAN Arduino library, which includes a part of the code from this repository, I made a couple of small changes to it, to fix warnings, documentation and ease debugging.

I've included these changes in this pull request. They are essentially three independent changes, put into separate commits. If needed, I can split them out into separate pull requests, but I thought it might be better to first show these and let you consider if you want to include any or all of them at all before I go to that additional effort.

This is a resubmit of #57, but I confirmed permission to submit this contribution from my contact at ST, so I now signed the individual CLA.

It seems the comment for GetDevAddress was copied from GetUniqueId, but
not changend sufficiently to be correct.
These warnings are caused because frequencies are stored as `uint32_t`,
which is `unsigned long`, while the printf format expects `int`. In
practice, this does not actually cause problems, since on STM32 gcc
`long` and `int` are both 32-bits and frequencies are never large enough
to cause signed vs unsigned ambiguity.

Since printf has no format specifiers for e.g. uint32_t (libc does have
some macros for this, but those really hurt readability), this is tricky
to fix in a portable way (other architectures or compilers might have
`uint32_t` equal to `unsigned int` instead of `unsigned long`), this fix
just casts the frequency to `unsigned` before passing it to printf (and
for good measure, also convert the specifier from `%d` to `%u`). This
does mean this printing will break if `int` is not at least 32-bits
(e.g. on AVR), but given the scope of this library, that should be
acceptable.
This adds some more details about RX and TX bytes and TX config. There
is also some code for RX config, but to minimize the impact on RX
timing, that is commented out.
@RJMSTM RJMSTM added the lorawan LoraWAN-related issue or pull-request. label Mar 29, 2023
@RJMSTM
Copy link
Contributor

RJMSTM commented Mar 29, 2023

Hello @matthijskooijman

Thank you for this report. We will get back to you as soon as we analyze it further. This may take some time. Thank you for your comprehension.

With regards,
Rania

@ASELSTM
Copy link
Contributor

ASELSTM commented May 17, 2023

ST Internal Reference: 152303

@ASELSTM ASELSTM added the internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system label May 17, 2023
@ALABSTM ALABSTM added bug Something isn't working mw Middleware-related issue or pull-request. labels Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system lorawan LoraWAN-related issue or pull-request. mw Middleware-related issue or pull-request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants