search

mcci-catena / arduino-lorawan

User-friendly library for using arduino-lmic with The Things Network and other LoRaWAN® networks
MIT License
274 stars 54 forks source link

Updated ESP32 Example - Backport from matthias-bs/BresserWeatherSensorTTN with fixes and additional features #211

Open matthias-bs opened 1 year ago

matthias-bs commented 1 year ago

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.)

matthias-bs commented 1 year ago

Updated arduino_lorawan_esp32_example to use any LoRaWAN network supported by the library.

terrillmoore commented 1 year ago

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:

https://github.com/mcci-catena/arduino-lorawan/pull/211

Commit Summary

File Changes

(1 https://github.com/mcci-catena/arduino-lorawan/pull/211/files file)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/mcci-catena/arduino-lorawan/pull/211 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AETP733GQB3JR24PLOHNVE3W23TL7ANCNFSM6AAAAAAVSD65MU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

matthias-bs commented 1 year ago

Moved example sketch modifications to separate PR as proposed.

terrillmoore commented 1 year ago

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:

  1. change line 37 of Arduino_LoRaWAN.h to: 
    ARDUINO_LORAWAN_VERSION_CALC(0, 10, 0, 1) /* v0.10.0-pre1 */
  2. Also change library.properties to match and the change log in README.md.
  3. send the PR
  4. get the PR merged
  5. Iterate with any other changes, incrementing the pre-release field.
  6. Send another PR requesting release of 0.10.0, and changing line 37 to: 
    ARDUINO_LORAWAN_VERSION_CALC(0, 10, 0, 0) /* v0.10.0 */
    1. Find previous release tickets and do the other work indicated there (tweaking badges, etc.)
    2. The merge of the release PR is accompanied by creating a release tag on github; which will cause users of the Arduino tooling to get updated.
matthias-bs commented 1 year ago

I tried to follow this procedure in https://github.com/mcci-catena/arduino-lorawan/pull/214

terrillmoore commented 1 year ago

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.

matthias-bs commented 1 year ago

Please merge #214 first. Then I will update #215. Then I will try to modify #211 maintain backwards-compatibility. Thanks in advance!

terrillmoore commented 1 year ago

OK, #214 is merged. For #211, since it's officially pre-release and we're bumping the version number, no need to really worry; go ahead and take out the prints.

matthias-bs commented 10 months ago

ESP32 Example - Backport from matthias-bs/BresserWeatherSensorTTN with fixes and additional features

matthias-bs commented 10 months ago

New Features