Correct ChannelCheckTimerEvent to send when tx NOT running #8
Conversation
…en MAC_TX_RUNNING bit is NOT set
|
I'm sorry, but, I think that the implementation is correct. The LoRaMacState variable reflects the current MAC layer state. The MAC_TX_RUNNING state indicates that a Class A transmission cycle is running. Could you please explain why you think the implementation is incorrect? |
|
I found that when a channel that is otherwise free to use (e.g. it is supported by the active bands, duty cycle restriction checks out ok, etc) has too high of a noise level (i think the Lib's current limit is around -90 rssi), the channelcheck event will call and reschedule itself as intended until the channel is free AND the noise ceiling is met. i believe the intention is then that the channelcheck event will send on the channel if it passes the mac_tx_running check, but it never does because the check currently only passes if the transmit/receive sequence has started, which doesn't happen at that stage yet. by setting the check to pass only if the transmit/receive sequence has NOT started, the channelcheck event will successfully start and complete the transmit/receive cycle. hope this helps, I'm only on my phone now but I'm happy to provide more info if needed later On July 9, 2015 3:15:15 PM GMT+03:00, Miguel Luis notifications@github.com wrote:
|
|
Ok, I see your point. The issue is that the patch may solve the problem at the first check before the MAC layer state is in MAC_TX_RUNNING but, it will produce an issue when the MAC layer is in MAC_TX_RUNNING state. I think that this issue needs further investigation. While we haven't found a better solution we could as a temporary solution remove the channel free check. The MAC layer is now respecting the ETSI duty cycle enforcement thus there is no need to check if the channel is free or not. // GitHub pull request #8 temporary solution
//if( Radio.IsChannelFree( MODEM_LORA, Channels[channelNext].Frequency, RSSI_FREE_TH ) == true )
{
// Free channel found
Channel = channelNext;
LoRaMacState &= ~MAC_CHANNEL_CHECK;
TimerStop( &ChannelCheckTimer );
return 0;
}If you agree could you please resubmit a patch with this change. |
|
Timer ChannelCheckTimer is started only by the function LoRaMacSetNextChannel() if there is no free channel. So why not just remove the check of MAC_TX_RUNNING in function OnChannelCheckTimerEvent() and lock LoRaMacSendFrameOnChannel for other layer? |
|
@mluis1 : Can the ChannelCheckTimer event ever actually fire after the MAC state transitions into MAC_TX_RUNNING? As far as I can tell, the ChannelCheckTimer event is only used to check if a channel is free after a transmission is denied due to no free channels being available, which means that the transmit/receive cycle is never initiated until the event's last callback. But even if this is true, I think you're right and this needs additional investigation, because LoRaMacSendOnChannel and its own calls like LoRaMacPrepareFrame are never invoked in the sequence (ChannelCheckTimer event "skips" ahead to LoRaMacSendFrameOnChannel) which means the actual frame being sent might be the last one prepared, not the present one requested. So I would not only agree with your above suggestion of removing the RSSI check but also suggest removing the use of ChannelCheckTimer and its callback until a better solution is found. |
|
We can't simply remove the ChannelCheckTimer. The duty cycle enforcement and acknowledgement retries relies on it. Once the MAC layer is in MAC_TX_RUNNING state it will try to send the last requested frame up until a channel becomes available. The LoRaMacSendFrameOnChannel must be kept accessible to the upper layers for test purposes. Example radio coverage test. When I get some time I will try to figure out a better solution that suits every one. |
|
I think that the stack keeps the LoRaMacEventFlags.Bits.Tx as long as the RX window is not ended. For my device I temporary patched on this way, but I'm not sure if it is a clean solution because I do not have time to fully investigate on the code. by addind the stuff commentedwith ///DWY-ADDED-CLASSC-PATCH-TEST if( IsRxWindowsEnabled == true ) |
Add define for RX1 and RX2 windows. Add some documentation. Add delay between two emission of the same unconfirmed frame. Add timer to manage join request. Remove check of MAC_TX_RUNNING in OnChannelCheckTimerEvent() to correct temporary PullRequest Lora-net#8. Remove MacStateCheckTimer. Rename and reorganize function OnMacStateCheckTimerEvent(). Class C : Open Rx2 when Class C device join a fake network LoRaMacInitNwkIds(), for a real connection it's manage by LoRaMacEndOfTxRxManager(). Continuous Rx2 is open after handling information. Change LoRaMacEventInfoStatus_t gestion : add more information about each case. Change LoRaMacState gestion : MAC_TX_RUNNING is set at the beginning of an emission cycle. is reset at the end of an emission cycle. Change LoRaMacEventFlags_t gestion : Tx is for all emission, successful, with error or on timeout. Rx is for all reception, successful (data or not), with error or on timeout. RxData is only for reception with data. Change gestion for UnConfirmed and Confirmed frame Unconfirmed : Reset ChannelsNbRepCounter before sending the frame for the first time in LoRaMacSendOnChannel. Add function LoRaMacManageUnconfirmedUplink to manage emission counter and end of emission. For multiple emission, the next one is done after the end of a cycle, Rx1 with error or Rx2 for ClassA and Rx1 for ClassC. Confirmed : Add function LoRaMacManageConfirmedUplink to manage end of emission cycle, successful or unsuccessful. Move the launch of AckTimeoutTimer. It was generating error when there was a reception on Rx1 with a Mic error, RxWindow2Timer was stop so it didn't launch the AckTimeoutTimer to send again the confirmed frame (For Class A devices). AckTimeoutTimer is now launch after an emission of a Confirmed frame, for Class A and C device. Why is the LoRaWan implemantation use a timer to notified the upper layer only when every operation is finished ? A timer add a delay between the last operation and the notification. It become impossible to use real time application in this case. Wouldn't it be better to use the timer callback as a function called after each radio event. In this way, every information would be forwarded to users and they will have to choose which to use or ignore. The utilisation of the structure LoRaMacEventFlags_t and more precisely the Bits.Tx can generate some error with class C devices. It was working well with class A because for every reception the device has to have an emission before. But for class C it can receive message without emission and the field Bits.Tx will be true. Just try the case where a class C device receive a frame, the mic is wrong, field Bits.Tx become true. Now imagine the previous configuration was NodeAckRequested false, and ChannelsNbRepCounter is inferior to ChannelsNbRep, the device will enter this part of the code and try to send a previous frame again, or there is no previous frame to send again: else { LoRaMacEventFlags.Bits.Tx = 0; // Sends the same frame again if( LoRaMacSetNextChannel( ) == 0 ) { LoRaMacSendFrameOnChannel( Channels[Channel] ); } } My point is, various field of LoRaMacEventsFlags should be used accordingly to their name to differenciate every case, Tx for an emission (succesful or unsuccessful), Rx for every reception successfull/failed/timeout. I think others are used as they should. Reorganisation of the code to follow this idea would reorganised the timer callback OnMacStateCheckTimerEvent to differenciate emission and reception. Actual gestion of confirmed frame can generate issue in Class C device. The timer AckTimeoutTimer is launched in OnRxWindow2TimerEvent, with a time between 1 and 3 seconds. If the delay is 1 second, Rx1 will not open and the device will send the frame again. The issue will occured if the gateway try to answer the confirmed frame on Rx1 in this case. Class C device should try to send again if nothing is received after Rx1.
ChannelCheckTimerEvent should call LoRaMacSendFrameOnChannel when free channel found and MAC_TX_RUNNING bit is NOT set