LoRa: Stack cleanup #6566
LoRa: Stack cleanup #6566
Conversation
- This implements IOTCELL-697 - This touches API, but does not break it, old ones still work in a same manner!
- Internal change only
| { | ||
| if (params) { | ||
| if (is_otaa) { | ||
| // if ((params->connection_u.otaa.dev_eui == NULL) || |
| // network server that the device was disconnected or restarted. | ||
| // At the moment, this implementation does not support a non-volatile | ||
| // memory storage. | ||
| //_lw_session.downlink_counter; //Get from NVM |
There was a problem hiding this comment.
why are these 2 lines here?
There was a problem hiding this comment.
This is a reminder for NVM support. It will be added later for LoRa
|
@hasnainvirk, @kivaisan, @kjbracey-arm Can you review this? |
| */ | ||
| void join_by_abp(const lorawan_connect_abp_t& abp_join); | ||
| lorawan_status_t join(bool is_otaa = true); |
There was a problem hiding this comment.
I think is_otaa should not have a default value. Defaulting to OTAA can enable easy mistake in calling this method.
There was a problem hiding this comment.
The same comment applies to prepare_join method as well.
| #include "lorawan/lorastack/phy/LoRaPHYUS915Hybrid.h" | ||
| #define LoRaPHY_region LoRaPHYUS915Hybrid | ||
| #else | ||
| #error "Unsupported region, check your configuration." |
There was a problem hiding this comment.
Unfortunately this won't work, because any unknown identifier will be interpreted as 0, so it will pass the #if MBED CONF_LORA_PHY == EU868 test.
You need the identifiers you recognise to be non-zero. Which is a bit unfortunate if you're trying to be backwards-compatible with having a meaning for 0.
- Also removed useless else from loraphy_target.h
|
Changed according to Kimmo's and Kevin's comments |
| #define KR920 7 | ||
| #define US915 8 | ||
| #define US915_HYBRID 9 | ||
|
|
There was a problem hiding this comment.
These end up permanently defined for any application code including LoRaWANStack.h. Again, maybe some concatenation could be done to allow short IDs in the JSON but not clutter C namespace. Could be done in future.
|
Region handling is now improved and also made sure it is still backwards compatible |
|
@0xc0170 Can you trigger the build for this? |
|
/morph build |
Build : SUCCESSBuild number : 1686 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 1318 |
Test : SUCCESSBuild number : 1480 |
|
@0xc0170 Can you merge this in now? |
Description
Refactored and cleaned up some internal code.
Tested manually by running tests locally
Pull request type
[ ] Fix
[ X] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change