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

fix #840: enable device time request by default #844

Merged
merged 4 commits into from
Jan 27, 2022
Merged

Conversation

dhineshkumarmcci
Copy link

No description provided.

Copy link
Member

@terrillmoore terrillmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, can you add a note to README.md, and also please change the version in lmic.h to something like 4.2.0-pre1? Thanks!

Copy link
Member

@terrillmoore terrillmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other thing -- please use git rebase -i and change the commit log message to be something like "Fix #840: Enable device time request by default" (it already was "enabled" in some sense, what you're changing is the default, so it will clarify to change the commit log, for tracking). (I change PR #844's title, but changing the commit log requires a git rebase -i to edit the commit log. Then you'll need to do a git push -f to issue840.)

@terrillmoore terrillmoore changed the title fix #840: enabled device time request fix #840: enable device time request by default Dec 31, 2021
Copy link
Member

@terrillmoore terrillmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do a search for LMIC_ENABLE_DeviceTimeReq.

  • It is used in ci/platformio.sh; we should remove it (now that it's the default).
  • We should add a test to confirm that LMIC_ENABLE_DeviceTimeReq=0 still compiles correctly.
  • We need to document the default state (README.md line 345). We should state "Prior to v4.2.0-pre1, the default was that device network-time requests were disabled. As of v4.2.0-pre1, the default is that device network-time requests are enabled."

@dhineshkumarmcci
Copy link
Author

* We should add a test to confirm that `LMIC_ENABLE_DeviceTimeReq=0` still compiles correctly.

Added tests with disabling device-time request, using ttn-otaa.ino for regions US and EU.

# Compile "ttn-otaa" example in US and EU with Device-time request disabled
PLATFORMIO_BUILD_FLAGS='-D CFG_us915 -D CFG_sx1276_radio -D LMIC_ENABLE_DeviceTimeReq=0 -D COMPILE_REGRESSION_TEST -D ARDUINO_LMIC_PROJECT_CONFIG_H_SUPPRESS' platformio ci --lib . --board heltec_wifi_lora_32 'examples/ttn-otaa/ttn-otaa.ino'
PLATFORMIO_BUILD_FLAGS='-D CFG_eu868 -D CFG_sx1276_radio -D LMIC_ENABLE_DeviceTimeReq=0 -D COMPILE_REGRESSION_TEST -D ARDUINO_LMIC_PROJECT_CONFIG_H_SUPPRESS' platformio ci --lib . --board heltec_wifi_lora_32 'examples/ttn-otaa/ttn-otaa.ino'

@terrillmoore terrillmoore merged commit 8d378ea into master Jan 27, 2022
@terrillmoore terrillmoore deleted the issue840 branch January 27, 2022 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants