Implement low-power support in the LoRa stack #1670
Merged
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
added 6 commits
March 1, 2019 16:12
…r code to use the HFXO manager. Use of timers in the LoRa stack has been split into critical and non-critial timers. Non-critical timers (such as retry delays) are now implemented using the os_cputime functions. The os_cputime needs to be configured to use a lower power time in the BSP (such as is done by the BLE stack via the BLE_LP_CLOCK syscfg). Critical timers are only used to time the opening of the RX windows. These continue to use the hal_timer functions, but require that the user implements two callback functions (lora_low_power_enter and lora_low_power_exit) to turn the LORA_MAC_TIMER off and on. This is necessary as turning a timer on is a hardware-specific operation that may require that the caller pass in a custom configuration struct. This patch also modifies the nRF timer code to use the HFXO manager found in hw/mcu/nordic/nrf52xxx/src/nrf5x_clock.c. This allows multiple different timers to be turned on and off independently and manager will automatically turn the nRF HFXO on and off as required. This is necessary in order for the LoRa and BLE stacks to be able to run side by side.
…mrbekhit/lora_low_power
The SX1272 DIO3 IRQ is used during Carrier Detect (CAD). This is neither necessary for LoRa operation, nor used in the current driver. Therefore, by making this pin optional, an extra GPIO/IRQ can be freed for other tasks.
wes3
requested changes
Mar 18, 2019
This something I tried to discuss on the mailing list https://lists.apache.org/thread.html/48451519a2a6990dfa98dea84ee4cf8e6c8b407e9220f152e9ab49a2@%3Cdev.mynewt.apache.org%3E Basically, the hal_timer functions don't provide a way for libraries to turn a timer on in a hardware agnostic fashion. You have to call the hal_timer_init function with a hardware specific parameter. Therefore, I declared the callbacks lora_low_power_enter/exit() that need to be implemented by the user in order to turn the timer on and off. I'd like to find a better way of resolving this. |
added 2 commits
March 18, 2019 15:12
…he#1670 (comment)). -Resolved minor formatting issues.
…mrbekhit/lora_low_power
wes3
requested changes
Apr 8, 2019
There was a problem hiding this comment.
So I reviewed the rest of the changes and they seem fine. However, I think what needs to be done to this PR is to separate out the nrf5x changes from the rest of the changes. They should be in separate PR's and not tied together. Furthermore, the changes as made will change the behavior of the code for folks who are already using it (there is no wait to see if the HFXO settles). So I think any changes in that separate PR need to have the HFXO wait to settle.
|
And another comment. Not sure how to handle the lora enter low power. Quite a few of the drivers require a hw specific config and this is usually handled in the bsp or in the architecture specific portion of drivers. So handling that in the lora hw specific driver seems reasonable. |
added 4 commits
April 26, 2019 07:04
…mrbekhit/lora_low_power
…se will be added to a separate pull request.
…an two: lora_bsp_enable_mac_timer. This is because the hal_timer functions already allow application code to disable timers in a hardware-agnostic fashion using the hal_timer_deinit function. The LoRa code now turns the timer off itself, but requires the BSP to implement lora_bsp_enable_mac_timer in order to turn the timer on.
…sp_enable_mac_timer function in order to allow CI to pass.
amrbekhit
changed the title
amrbekhit
changed the title
amrbekhit
commented
Apr 30, 2019
There was a problem hiding this comment.
Hi @wes3 as requested I've removed the HFXO changes to hal_system.c and hal_timer.c and will add these as a separate pull request. EDIT the new pull request can be found here.
I've made some changes to the low power callbacks - since the LoRa code can turn the timer off itself using hal_timer_deinit, I've removed the lora_low_power_enter callback function and renamed lora_low_power_exit to lora_bsp_enable_mac_timer. BSPs now only need to implement lora_bsp_enable_mac_timer.
I've also modified the b-l072z-lrwan1 and telee02 BSPs to implement lora_bsp_enable_mac_timer. Travis CI now builds find and this pull request passes all checks.
Hopefully this can now be merged.
|
@wes3 can you review this so we can merge? |
|
Reviewed the changes and merged. Sorry for the delay. |
pshah111
pushed a commit
to JuulLabs/mynewt-core
that referenced
this pull request
May 31, 2019
* Implement low-power support in the LoRa stack and modify the nRF timer code to use the HFXO manager. Use of timers in the LoRa stack has been split into critical and non-critial timers. Non-critical timers (such as retry delays) are now implemented using the os_cputime functions. The os_cputime needs to be configured to use a lower power time in the BSP (such as is done by the BLE stack via the BLE_LP_CLOCK syscfg). Critical timers are only used to time the opening of the RX windows. These continue to use the hal_timer functions, but require that the user implements two callback functions (lora_low_power_enter and lora_low_power_exit) to turn the LORA_MAC_TIMER off and on. This is necessary as turning a timer on is a hardware-specific operation that may require that the caller pass in a custom configuration struct. This patch also modifies the nRF timer code to use the HFXO manager found in hw/mcu/nordic/nrf52xxx/src/nrf5x_clock.c. This allows multiple different timers to be turned on and off independently and manager will automatically turn the nRF HFXO on and off as required. This is necessary in order for the LoRa and BLE stacks to be able to run side by side. * Correct HFXO manager header files. * Correct calls to to HFXO manager. * Correct HFXO manager header. * Make SX1272 DIO3 IRQ optional. The SX1272 DIO3 IRQ is used during Carrier Detect (CAD). This is neither necessary for LoRa operation, nor used in the current driver. Therefore, by making this pin optional, an extra GPIO/IRQ can be freed for other tasks. * -Fixed a bug when calculating the delays for the TX retry timer (apache#1670 (comment)). -Resolved minor formatting issues. * -Reverted the HFXO changes in the hal_system and hal_timer files. These will be added to a separate pull request. * -Modified the low power callbacks so that there is only one rather than two: lora_bsp_enable_mac_timer. This is because the hal_timer functions already allow application code to disable timers in a hardware-agnostic fashion using the hal_timer_deinit function. The LoRa code now turns the timer off itself, but requires the BSP to implement lora_bsp_enable_mac_timer in order to turn the timer on. * -Modified the b-l072z-lrwan1 and telee02 BSPs to implement the lora_bsp_enable_mac_timer function in order to allow CI to pass.
amrbekhit
added a commit
to amrbekhit/mynewt-core
that referenced
this pull request
Jul 14, 2019
* Implement low-power support in the LoRa stack and modify the nRF timer code to use the HFXO manager. Use of timers in the LoRa stack has been split into critical and non-critial timers. Non-critical timers (such as retry delays) are now implemented using the os_cputime functions. The os_cputime needs to be configured to use a lower power time in the BSP (such as is done by the BLE stack via the BLE_LP_CLOCK syscfg). Critical timers are only used to time the opening of the RX windows. These continue to use the hal_timer functions, but require that the user implements two callback functions (lora_low_power_enter and lora_low_power_exit) to turn the LORA_MAC_TIMER off and on. This is necessary as turning a timer on is a hardware-specific operation that may require that the caller pass in a custom configuration struct. This patch also modifies the nRF timer code to use the HFXO manager found in hw/mcu/nordic/nrf52xxx/src/nrf5x_clock.c. This allows multiple different timers to be turned on and off independently and manager will automatically turn the nRF HFXO on and off as required. This is necessary in order for the LoRa and BLE stacks to be able to run side by side. * Correct HFXO manager header files. * Correct calls to to HFXO manager. * Correct HFXO manager header. * Make SX1272 DIO3 IRQ optional. The SX1272 DIO3 IRQ is used during Carrier Detect (CAD). This is neither necessary for LoRa operation, nor used in the current driver. Therefore, by making this pin optional, an extra GPIO/IRQ can be freed for other tasks. * -Fixed a bug when calculating the delays for the TX retry timer (apache#1670 (comment)). -Resolved minor formatting issues. * -Reverted the HFXO changes in the hal_system and hal_timer files. These will be added to a separate pull request. * -Modified the low power callbacks so that there is only one rather than two: lora_bsp_enable_mac_timer. This is because the hal_timer functions already allow application code to disable timers in a hardware-agnostic fashion using the hal_timer_deinit function. The LoRa code now turns the timer off itself, but requires the BSP to implement lora_bsp_enable_mac_timer in order to turn the timer on. * -Modified the b-l072z-lrwan1 and telee02 BSPs to implement the lora_bsp_enable_mac_timer function in order to allow CI to pass.
brolan-juul
pushed a commit
to JuulLabs/mynewt-core
that referenced
this pull request
Oct 17, 2019
* Implement low-power support in the LoRa stack and modify the nRF timer code to use the HFXO manager. Use of timers in the LoRa stack has been split into critical and non-critial timers. Non-critical timers (such as retry delays) are now implemented using the os_cputime functions. The os_cputime needs to be configured to use a lower power time in the BSP (such as is done by the BLE stack via the BLE_LP_CLOCK syscfg). Critical timers are only used to time the opening of the RX windows. These continue to use the hal_timer functions, but require that the user implements two callback functions (lora_low_power_enter and lora_low_power_exit) to turn the LORA_MAC_TIMER off and on. This is necessary as turning a timer on is a hardware-specific operation that may require that the caller pass in a custom configuration struct. This patch also modifies the nRF timer code to use the HFXO manager found in hw/mcu/nordic/nrf52xxx/src/nrf5x_clock.c. This allows multiple different timers to be turned on and off independently and manager will automatically turn the nRF HFXO on and off as required. This is necessary in order for the LoRa and BLE stacks to be able to run side by side. * Correct HFXO manager header files. * Correct calls to to HFXO manager. * Correct HFXO manager header. * Make SX1272 DIO3 IRQ optional. The SX1272 DIO3 IRQ is used during Carrier Detect (CAD). This is neither necessary for LoRa operation, nor used in the current driver. Therefore, by making this pin optional, an extra GPIO/IRQ can be freed for other tasks. * -Fixed a bug when calculating the delays for the TX retry timer (apache#1670 (comment)). -Resolved minor formatting issues. * -Reverted the HFXO changes in the hal_system and hal_timer files. These will be added to a separate pull request. * -Modified the low power callbacks so that there is only one rather than two: lora_bsp_enable_mac_timer. This is because the hal_timer functions already allow application code to disable timers in a hardware-agnostic fashion using the hal_timer_deinit function. The LoRa code now turns the timer off itself, but requires the BSP to implement lora_bsp_enable_mac_timer in order to turn the timer on. * -Modified the b-l072z-lrwan1 and telee02 BSPs to implement the lora_bsp_enable_mac_timer function in order to allow CI to pass.
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.
Use of timers in the LoRa stack has been split into critical and non-critial timers. Non-critical timers (such as retry delays) are now implemented using the os_cputime functions. The
os_cputime needs to be configured to use a lower power time in the BSP (such as is done by the BLE stack via the BLE_LP_CLOCK syscfg). Critical timers are only used to time the opening of
the RX windows. These continue to use the hal_timer functions, but require that the user implements two callback functions (lora_low_power_enter and lora_low_power_exit) to turn the
LORA_MAC_TIMER off and on. This is necessary as turning a timer on is a hardware-specific operation that may require that the caller pass in a custom configuration struct.
This patch also modifies the nRF timer code to use the HFXO manager found in hw/mcu/nordic/nrf52xxx/src/nrf5x_clock.c. This allows multiple different timers to be turned on and offEDIT: As requested, this has now been removed and will be pushed as a separate pull request.independently and manager will automatically turn the nRF HFXO on and off as required. This is necessary in order for the LoRa and BLE stacks to be able to run side by side.