Updated ESP32 Example - Backport from matthias-bs/BresserWeatherSensorTTN with fixes and additional features #211
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Updated arduino_lorawan_esp32_example to use any LoRaWAN network supported by the library. |
|
Thanks for your contribution. Although I agree that this is an improvement, it will cause changes in the case of people who are using this facility (output will change).
Either we have to bump the version number (at least increase the minor version, as this is arguably not a breaking change; or possibly the major version, as it may break things), or figure out a way to make this work properly for new sketches that are aware of the change, and continue to work as at present for sketches that are not. I’m on a very slow network connection at the moment, so I can’t really study the diffs and see if there might be a clever refactoring (possibly adding some configuration property to `myEventLog()` that can be queried to suppress the calls – if it’s a compile-time constant, an `if` will be completely removed when false). But even this will require a minor bump, because it’s new functionality.
I think this also means that the example-sketch change should be made in a different PR.
Thanks,
…--Terry
From: Matthias Prinke ***@***.***>
Sent: Tuesday, March 7, 2023 03:01
To: mcci-catena/arduino-lorawan ***@***.***>
Cc: Subscribed ***@***.***>
Subject: [mcci-catena/arduino-lorawan] Removed Serial.print() calls in Arduino_LoRaWAN::cEventLog::processSingleEvent() (PR #211)
If debug output is implemented with a callback (myEventLog.logEvent()), its contents, formatting and debug port should entirely be defined there. Specifically, I am using the logging macros from https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-log.h in my logEvent() implementation.
Currently, there is still some output from the library itself, which results in some minor inconsistencies.
E.g.
10:42:46.290 -> [ 47074][D][BresserWeatherSensorTTN.ino:820] NetTxComplete():
10:42:46.290 -> 41359 ms:[ 47090][I][BresserWeatherSensorTTN.ino:772] operator()(): TX @41359 ms: ch=2 rps=0x03 (SF9 BW125 CR 4/5 Crc IH=0)
10:42:46.290 ->
(Preceding extra timestamp 41359ms:, extra line-feed.)
_____
You can view, comment on, or merge this pull request online at:
#211
Commit Summary
* 9871f07 <9871f07> Removed Serial.print() calls
File Changes
(1 <https://github.com/mcci-catena/arduino-lorawan/pull/211/files> file)
* M src/lib/arduino_lorawan_cEventLog.cpp <https://github.com/mcci-catena/arduino-lorawan/pull/211/files#diff-43b0009e97fc758708f0fa5b52c8b89c7aa87e9a3267954212565257814893c4> (3)
Patch Links:
* https://github.com/mcci-catena/arduino-lorawan/pull/211.patch
* https://github.com/mcci-catena/arduino-lorawan/pull/211.diff
—
Reply to this email directly, view it on GitHub <#211> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AETP733GQB3JR24PLOHNVE3W23TL7ANCNFSM6AAAAAAVSD65MU> .
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
|
Moved example sketch modifications to separate PR as proposed. |
|
I forgot that I never moved this library to V1, so we don't have to do a major bump (not required by semver if version is 0). However, we should change the version to 0.10. The way to do this in a pr is:
|
|
I tried to follow this procedure in #214 |
|
Yes, thanks, I saw. Let me know which PR you'd like to merge first, and let me know when you're done on a given PR; don't want do to things in the wrong order or prematurely. You'll probably have to rebase your other branches (for the version change) as we move forward, but that's relatively painless. |
|
ESP32 Example - Backport from matthias-bs/BresserWeatherSensorTTN with fixes and additional features |
matthias-bs
changed the title
New Features
|
…duino-lorawan into esp32_example_new
…duino-lorawan into esp32_example_new
Replaced DEBUG_PRINTF/DEBUG_PRINTF_TS by macros log_i/../log_d/log_v Added support for Adafruit Feather RP2040 with RFM95W FeatherWing Renamed FIREBEETLE_COVER_LORA in FIREBEETLE_ESP32_COVER_LORA Improved config for Firebeetle Cover LoRa and Adafruit Feather ESP32-S2 (default battery voltage thresholds)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If debug output is implemented with a callback (
myEventLog.logEvent()), its contents, formatting and debug port should entirely be defined there. Specifically, I am using the logging macros from https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-log.h in mylogEvent()implementation.Currently, there is still some output from the library itself, which results in some minor inconsistencies.
E.g.
(Preceding extra timestamp
41359ms:, extra line-feed.)