g4klx / MMDVMHost

The host program for the MMDVM
GNU General Public License v2.0
379 stars 274 forks source link

Dstar m_rfTimeoutTimer is never stopped under some circumstances causing lost transmission #756

Open KK7BYQ opened 1 year ago

KK7BYQ commented 1 year ago

Issue Description

There is an issue where the m_rfTimeoutTimer doesn't get stopped at the end of a D-star RF transmission causing the next subsequent transmission to be lost (if the next transmission occurs after the rfTimeout time).

Because the m_rfTimeoutTimer is stopped in the code that sends the Ack, if data is received from the network between the end of an RF transmission but prior to the m_ackTimer firing, the ackTimer is stopped and the m_rfTimeoutTimer will continue. If there isn't a subsequent RF transmission within the m_rfTimeoutTimer configuration time (default 240/4 mins), the rf timer will be flagged as expired and the next subsequent transmission will not be sent to the network. This can happen during a busy net check-in where network traffic is likely to be received within the default ack time of 750ms after the initial RF transmission and there isn't a subsequent RF transmission within the rf timeout. (operator waits more than 4 minutes for their turn)

Steps to reproduce.

For easier testing make the following configuration changes in MMDVMhost expert configuration:

General Timeout: 10 D-Star AckReply: 1 D-Star AckTime: 1000

Connect to REF55E or any network echo. (I didn't test if local echo will cause this)

Transmit a test call. Verify that the echo server returns the response and stomps on the ACK beep. (no ack beep heard)

Wait at least 10 seconds and repeat the transmission. Note that the dashboard receives the RF but does not send to the network.

Pertinent Code in DStarControl.cpp

line 639


void CDStarControl::writeEndRF()
{
    m_rfState = RS_RF_LISTENING;

    if (m_netState == RS_NET_IDLE) {
        m_display->clearDStar();

        m_ackTimer.start(); //<---------------ack start after end of transmission

        if (m_network != NULL)
            m_network->reset();
    } else {
        m_rfTimeoutTimer.stop(); //<--------------- not called
    }
}

line 1155

void CDStarControl::sendAck()
{
    m_rfTimeoutTimer.stop(); //<--------------- m_rfTimeoutTimer is only stopped after m_ackTimer has fired.
...

Line 713

void CDStarControl::writeNetwork()
{
...
m_ackTimer.stop(); //<--------------- New header is received from network and m_ackTimer is stopped, but m_rfTimeoutTimer continues
...

Possible fix.

If an EOT is received, stop m_rfTimeoutTimer before any other activity.

void CDStarControl::writeEndRF()
{   
    m_rfTimeoutTimer.stop(); //<--------------- stop rf timer immediately
    m_rfState = RS_RF_LISTENING;

    if (m_netState == RS_NET_IDLE) {
        m_display->clearDStar();

        m_ackTimer.start();

        if (m_network != NULL)
            m_network->reset();
    }
       //else { <--------------- remove else
        //m_rfTimeoutTimer.stop();
    //}
}

Workaround

A work around is to set the AckTime to 10ms (thus reducing the chance that a network transmission can sneak in before the ack). Also setting the General Timeout to a higher number would make it more likely that the next RF transmission will happen before the rfTimeout happens. But neither of these eliminates the possibililty.

g4klx commented 1 year ago

This is a lovely description of the bug, and your fix makes complete sense too. I have implemented it in the master branch. However I can see a problem with your fix also. The reason for the lack of a timeout stop in that case was to ensure that another RF user cutting in before the ack would only have the what was left of the timeout to use. This would strongly encourage the RF users to wait for the ack before transmitting.

Maybe a better fix would be revert your (and now my) changes, and to stop the RF timeout at the beginning of an incoming network transmission. This would keep the RF timeout behaviour, but would allow for better handling of RF users. I would like to think that network users would be less aggressive with their PTT usage.

What do you think?

Jonathan G4KLX

KK7BYQ commented 1 year ago

Hi Jonathan, Thanks for responding. Stopping the RF timeout in the network header at approximately line 713? That seems like a good option. Would someone being too quick on the PTT even know why their transmission was cut off? I’m good with either solution and just happy I could help.

Thanks, Byron KK7BYQ