Improving ACK in sketch PiGateway.ino vs Gateway.ino

Started by Sergegsx, December 04, 2015, 08:04:59 AM

Sergegsx

Hello,
I have been trying to get full reliability when requesting ACK from a Node to the Gateway. What I have found out is that in Gateway.ino I can get a very high reliability whilst in the PiGateway.ino it was sometimes near none ACK received.

I have noticed that these two sketchs differ in the following:

Gateway.ino   https://github.com/LowPowerLab/RFM69/blob/master/Examples/Gateway/Gateway.ino
void loop() {
  //process any serial input
  if (Serial.available() > 0)
  {
    char input = Serial.read();
etc...



PiGateway.ino

void loop() {
  inputLen = readSerialLine(input);
  inputstr = String(input);
  inputstr.toUpperCase();
  
  if (inputLen > 0)
  {
etc...


by modifying the PiGateway.ino to check if there is Serial data available, the PiGateway will now work very good and get around 96% of ACK with an ok.

void loop() {
  if (Serial.available()>0){
  inputLen = readSerialLine(input);
  inputstr = String(input);
  inputstr.toUpperCase();
  
  if (inputLen > 0)
  {
etc...


Anyone has noticed this? Any reason nobody else has reported it (I think)?

Thanks

Jferns

I had the same issue with Acks in PiGateway.ino. With changes similar to what you have listed I get Acks from gateway to nodes correctly.

readSerialLine()  perhaps takes a longer time to respond than Serial.available().

Sergegsx

I have also noticed that ACK might be working 100% at some times, and then few hours later, same code, same hardware, all running continuously, the ACK success drops a lot !

Any idea why?

TomWS

Quote from: Sergegsx on December 05, 2015, 08:22:37 AM
I have also noticed that ACK might be working 100% at some times, and then few hours later, same code, same hardware, all running continuously, the ACK success drops a lot !

Any idea why?
for a node to receive an ACK if using sendWithRetry(), the ACK must be sent within the time allotted to the retry check, which, I believe, defaults to 40mS.  If you also use the default of 3 retries, then you might receive the ACK from the first transmission within 120mS.  This timing might be too tight with a Linux based Gateway, especially if its busy doing some other scheduled task.

In general, a receiving node (gateway or otherwise) should respond with an ACK before doing anything with the received data.

Tom

Sergegsx

#4
Thanks Tom.

The strange thing is that I am running the PiGateway.ino code as in Felix github. As far as the python code reading through serial, I dont think it should affect it. When the PiGateway.ino receives a message from one of the nodes, it will send the ACK back without there being anything that should block it.

This is an extract of the PiGateway.ino. There is nothing blocking once a message is received apart of CheckForWirelessHEX

if (radio.receiveDone())
  {
    int rssi = radio.RSSI;
    DEBUG('[');DEBUG(radio.SENDERID);DEBUG("] ");
    if (radio.DATALEN > 0)
    {
      for (byte i = 0; i < radio.DATALEN; i++)
        DEBUG((char)radio.DATA[i]);
      DEBUG("   [RSSI:");DEBUG(rssi);DEBUG("]");
    }

    CheckForWirelessHEX(radio, flash, false); //non verbose DEBUG

    if (radio.ACKRequested())
    {
      byte theNodeID = radio.SENDERID;
      radio.sendACK();
      DEBUG("[ACK-sent]");
    }
    DEBUGln();
    Blink(LED,3);
  }


So although I totally understand the ACK time variable, I dont understand why my nodes dont get *ACK ok* all of the times, and moreover, sometimes there is a good period where I get many "ok" and sometimes there are bad periods with mainly "nothing..."


also this code is as in original, so I guess noone else is having this problem?


edit...

in terms of one of the nodes. There is also nothing blocking communication. This is the code extract:

// *********************** SEND MSG VIA RFM69 ***********************************************
 byte buffLen=strlen(payload);
 delay(5);
 if (radio.sendWithRetry(GATEWAYID, payload, buffLen)) {
        Serial.print(" ok!");
        success++;
        }
      else Serial.print(" nothing...");

  Serial.println();
// ************************ SEND MSG VIA RFM69 **********************************************

TomWS

I suggest you change the code to:
if (radio.receiveDone())
  {
// UPDATE: Forgot to save before calling sendACK() - this is IMPORTANT!
    datalen = radio.DATALEN;
    if (datalen)
      memcpy(rx_buf,(const void *)radio.DATA,datalen);  // save data from device
    sender  = radio.SENDERID;
    target  = radio.TARGETID;
    rssi    = radio.RSSI;
    ackReq  = radio.ACKRequested();

    if (radio.ACKRequested())
    {
      byte theNodeID = radio.SENDERID;
      radio.sendACK();
      DEBUG("[ACK-sent]");
    }

    int rssi = radio.RSSI;
    DEBUG('[');DEBUG(radio.SENDERID);DEBUG("] ");
    if (radio.DATALEN > 0)
    {
      for (byte i = 0; i < radio.DATALEN; i++)
        DEBUG((char)radio.DATA[i]);
      DEBUG("   [RSSI:");DEBUG(rssi);DEBUG("]");
    }

    CheckForWirelessHEX(radio, flash, false); //non verbose DEBUG  TWS: Is this really needed in a Gateway Node???

    DEBUGln();
    Blink(LED,3);
  }


You will have far more success...

Tom

Sergegsx

It is a good idea, already testing it.

However, big problem, it seems that after doing the "radio.sendACK();" the information received by radio is lost and cant be accessed by radio.SENDERID or radio.SENDERID or radio.DATALEN

Any thoughts?

Thanks Tom

TomWS

Quote from: Sergegsx on December 05, 2015, 10:19:44 AM
It is a good idea, already testing it.

However, big problem, it seems that after doing the "radio.sendACK();" the information received by radio is lost and cant be accessed by radio.SENDERID or radio.SENDERID or radio.DATALEN

Any thoughts?

Thanks Tom
Yeah, sorry about that.  See my updated post.  I realized as soon as I posted the first answer.

Sergegsx

#8
just to get it right, what datatype for the new variables? and the rx_buf?

i have tried this but rx_buf gives error

  if (radio.receiveDone())
  {
   // UPDATE: Forgot to save before calling sendACK() - this is IMPORTANT!
    uint8_t ackReq  = radio.ACKRequested();
    char* rx_buf;
    uint8_t datalen = radio.DATALEN;
    if (datalen)
      memcpy(rx_buf,(const void *)radio.DATA,datalen);  // save data from device
    uint8_t sender  = radio.SENDERID;
    uint8_t target  = radio.TARGETID;
    uint8_t rssi    = radio.RSSI;
    

    if (radio.ACKRequested())
    {
      byte theNodeID = radio.SENDERID;
      radio.sendACK();
      DEBUG("[ACK-sent]");
    }

  //  int rssi = radio.RSSI;
    DEBUG('[');DEBUG(radio.SENDERID);DEBUG("] ");
    if (radio.DATALEN > 0)
    {
      for (byte i = 0; i < radio.DATALEN; i++)
        DEBUG((char)radio.DATA[i]);
      DEBUG("   [RSSI:");DEBUG(rssi);DEBUG("]");
    }

    
    CheckForWirelessHEX(radio, flash, false); //non verbose DEBUG

    DEBUGln();
    Blink(LED,3);
    }


thanks !

TomWS

Quote from: Sergegsx on December 05, 2015, 10:24:30 AM
just to get it right, what datatype for the new variables? and the rx_buf?

i have tried this but rx_buf gives error

  if (radio.receiveDone())
  {
   // UPDATE: Forgot to save before calling sendACK() - this is IMPORTANT!
    uint8_t ackReq  = radio.ACKRequested();
    char rx_buf[MAX_PACKET_SIZE];
    uint8_t datalen = radio.DATALEN;
    if (datalen)
      memcpy(rx_buf,(const void *)radio.DATA,datalen);  // save data from device
    uint8_t sender  = radio.SENDERID;
    uint8_t target  = radio.TARGETID;
    uint8_t rssi    = radio.RSSI;
    

    if (radio.ACKRequested())
    {
      byte theNodeID = radio.SENDERID;
      radio.sendACK();
      DEBUG("[ACK-sent]");
    }

  //  int rssi = radio.RSSI;
    DEBUG('[');DEBUG(radio.SENDERID);DEBUG("] ");
    if (radio.DATALEN > 0)
    {
      for (byte i = 0; i < radio.DATALEN; i++)
        DEBUG((char)radio.DATA[i]);
      DEBUG("   [RSSI:");DEBUG(rssi);DEBUG("]");
    }

    
    CheckForWirelessHEX(radio, flash, false); //non verbose DEBUG

    DEBUGln();
    Blink(LED,3);
    }


thanks !
See correction above.  You need to have space for the data, not an empty pointer to non-existent space...

Tom

Sergegsx

#10
thanks Tom
I have updated and corrected a few things, now it compiles.
- rssi must be an "int" / "int16_t"
- changed the print code to use our new variables instead of "radio."
- changed buffer size to "RF69_MAX_DATA_LEN" as per felix library

if (radio.receiveDone())
  {
   // UPDATE: Forgot to save before calling sendACK() - this is IMPORTANT!
    char rx_buf[RF69_MAX_DATA_LEN];
    uint8_t datalen = radio.DATALEN;
    if (datalen)
      memcpy(rx_buf,(const void *)radio.DATA,datalen);  // save data from device
    uint8_t sender  = radio.SENDERID;
    uint8_t target  = radio.TARGETID;
    int rssi    = radio.RSSI;
    uint8_t ackReq  = radio.ACKRequested();
    
    if (radio.ACKRequested())
    {
      byte theNodeID = radio.SENDERID;
      radio.sendACK();
      DEBUG("[ACK-sent]");
    }

  //  int rssi = radio.RSSI;
    DEBUG('[');DEBUG(sender);DEBUG("] ");
    if (datalen > 0)
    {
      for (byte i = 0; i < datalen; i++)
        DEBUG((char)rx_buf[i]);
      DEBUG("   [RSSI:");DEBUG(rssi);DEBUG("]");
    }

    
    CheckForWirelessHEX(radio, flash, false); //non verbose DEBUG

    DEBUGln();
    Blink(LED,3);
    }


I have added some counters to one of my nodes to keep track of how many ACK are lost and give me a %.
Lets see if we have improved!

After only 15 messages. I get 50% success. So far it did not make a difference. :(
I will report back later. thanks

Sergegsx

Can anyone tell me what are the implications of:

#define ACK_TIME       30 // # of ms to wait for an ack


Can it be increased freely? I cant find the code in the library or sketch that deals with this ack time, so I dont know how it can affect.

btw, I seem to have a "poltergeist" on top of my multi-function printer. As soon as I move the gateway arduino on top of one of its corners I start loosing ACK. If I move it 10cm away from it then 100% ACK!!! (and the printer is plug disconnected)

Isn't it amazing  :o
(see picture of the moment I move the gateway to on top of the printer!)

Felix

Quote from: Sergegsx on December 05, 2015, 11:29:48 AM
Can anyone tell me what are the implications of:

#define ACK_TIME       30 // # of ms to wait for an ack


Can it be increased freely? I cant find the code in the library or sketch that deals with this ack time, so I dont know how it can affect.

btw, I seem to have a "poltergeist" on top of my multi-function printer. As soon as I move the gateway arduino on top of one of its corners I start loosing ACK. If I move it 10cm away from it then 100% ACK!!! (and the printer is plug disconnected)

Isn't it amazing  :o
(see picture of the moment I move the gateway to on top of the printer!)

ACK time is somewhat linearly dependent on packet length. Usually ACKs are empty packages so a wait time of 30ms is around maximum. Normally they should be received in under 10ms. This figure is also dependent on bitrate, a very low bitrate will require longer to transmit the same packet, etc.

"Isn't it amazing" - yes very. It has to do with RF and antenna polarization.

Sergegsx

Thanks Felix. Just to learn a bit more, where is it used the value specified in the sketch? I cant find any reference to it anywhere apart of its definition in PiGateway.ino

I have lost more than a few hours just because the first time using the RFM69 I happened to leave the gateway on top of the printer!! :-[

Sergegsx

#14
Would you consider 96% of success in getting ACK a good value? I have 3 rfm69hw in the same room and separated 3m and 2m from the gateway.

I am pretty sure all the messages are arriving but ACK is around 96%. Is this acceptable? I was hoping a bit higher value