Garbage at end of received data.

Started by dave_sausages, August 28, 2016, 06:48:15 PM

dave_sausages

For some reason I'm receiving a string of random characters at the end of my received data.

When I run the standard unreliable client and server examples I get the text strings appearing in the serial monitor as normal, but when I send custom data I end up with an a random number of random characters on the end.

This is the module sending the custom data's code"
// rf95_client.pde
// -*- mode: C++ -*-
// Example sketch showing how to create a simple messageing client
// with the RH_RF95 class. RH_RF95 class does not provide for addressing or
// reliability, so you should only use RH_RF95 if you do not need the higher
// level messaging abilities.
// It is designed to work with the other example rf95_server
// Tested with Moteino-LoRa, MoteinoMEGA-LoRa

#include <SPI.h>
#include <RH_RF95.h>

#define FREQUENCY  434
#define FREQUENCY  915

#ifdef __AVR_ATmega1284P__
  #define LED           15 // Moteino MEGAs have LEDs on D15
  #define FLASH_SS      23 // and FLASH SS on D23
#else
  #define LED           9 // Moteinos have LEDs on D9
  #define FLASH_SS      8 // and FLASH SS on D8
#endif

// Singleton instance of the radio driver
RH_RF95 rf95;

byte sendLen;
char buffer[50];

int armed = 0; 
int dave = 123;
int dave2 = 456;
int dave3 = 789;


void setup() 
{
  Serial.begin(115200);
  if (!rf95.init())
    Serial.println("init failed");
  else { Serial.print("init OK - "); Serial.print(FREQUENCY); Serial.print("mhz"); }
  
  // Defaults after init are 434.0MHz, 13dBm, Bw = 125 kHz, Cr = 4/5, Sf = 128chips/symbol, CRC on
  rf95.setFrequency(FREQUENCY);
  //rf95.setFrequency(915);
}

void loop()
{
  
sprintf(buffer, "%d,%d,%d,%d", armed, dave, dave2, dave3);
    sendLen = strlen(buffer);  //get the length of buffer

rf95.send((uint8_t*)buffer, sendLen);
rf95.waitPacketSent();
Serial.print("sent");
Serial.println(buffer);
Serial.println(armed);
  
  rf95.waitPacketSent();
  // Now wait for a reply
  uint8_t buf[RH_RF95_MAX_MESSAGE_LEN];
  uint8_t len = sizeof(buf);

  if (rf95.waitAvailableTimeout(3000))
  { 
    // Should be a reply message for us now   
    if (rf95.recv(buf, &len))
   {
      Serial.print("got reply: ");
      Serial.println((char*)buf);
      Serial.print("RSSI: ");
      Serial.println(rf95.lastRssi(), DEC);    
    }
    else
    {
      Serial.println("recv failed");
    }
  }
  else
  {
    Serial.println("No reply, is rf95_server running?");
  }
  Blink(LED,3);
  delay(200);
}

void Blink(byte PIN, int DELAY_MS)
{
  pinMode(PIN, OUTPUT);
  digitalWrite(PIN,HIGH);
  delay(DELAY_MS);
  digitalWrite(PIN,LOW);
}


And this is the standard server code running on the other module:

// rf95_server.pde
// -*- mode: C++ -*-
// Example sketch showing how to create a simple messageing server
// with the RH_RF95 class. RH_RF95 class does not provide for addressing or
// reliability, so you should only use RH_RF95  if you do not need the higher
// level messaging abilities.
// It is designed to work with the other example rf95_client
// Tested with Moteino-LoRa, MoteinoMEGA-LoRa

#include <SPI.h>
#include <RH_RF95.h>

#define FREQUENCY  434
#define FREQUENCY  915

// Singleton instance of the radio driver
#ifdef __AVR_ATmega1284P__
  #define LED           15 // Moteino MEGAs have LEDs on D15
  #define FLASH_SS      23 // and FLASH SS on D23
#else
  #define LED           9 // Moteinos have LEDs on D9
  #define FLASH_SS      8 // and FLASH SS on D8
#endif

RH_RF95 rf95;

void setup() 
{
  pinMode(LED, OUTPUT);
  Serial.begin(115200);
  if (!rf95.init())
    Serial.println("init failed");
  else { Serial.print("init OK - "); Serial.print(FREQUENCY); Serial.print("mhz"); }
  // Defaults after init are 434.0MHz, 13dBm, Bw = 125 kHz, Cr = 4/5, Sf = 128chips/symbol, CRC on
  rf95.setFrequency(FREQUENCY);
}

void loop()
{
  if (rf95.available())
  {
    // Should be a message for us now   
    uint8_t buf[RH_RF95_MAX_MESSAGE_LEN];
    uint8_t len = sizeof(buf);
    if (rf95.recv(buf, &len))
    {
      digitalWrite(LED, HIGH);
//      RH_RF95::printBuffer("request: ", buf, len);
      Serial.print("got request: ");
      Serial.println((char*)buf);
      Serial.print("RSSI: ");
      Serial.println(rf95.lastRssi(), DEC);
      
      // Send a reply
      uint8_t data[] = "And hello back to you";
      rf95.send(data, sizeof(data));
      rf95.waitPacketSent();
      Serial.println("Sent a reply");
      digitalWrite(LED, LOW);
    }
    else
    {
      Serial.println("recv failed");
    }
  }
}



What is weird is that the "random" characters don't change during each power on. So the serial monitor output looks like this on one power on
got request: 0,123,456,789ÿ
RSSI: -45
Sent a reply
got request: 0,123,456,789ÿ
RSSI: -43
Sent a reply
got request: 0,123,456,789ÿ
RSSI: -42
Sent a reply
got request: 0,123,456,789ÿ
RSSI: -44
Sent a reply
got request: 0,123,456,789ÿ
RSSI: -44
Sent a reply
got request: 0,123,456,789ÿ


and on the next power on it will look like this:

got request: 0,123,456,789E[12].±¦[0F][08][07]1[0B][11]'QØ"d»%¡[0C]([01]¦)[18]æAM![17]†$¹4[08]Ù8œ;÷#£xÃ@—Šh6[0F]ýn"´%½M¹e;Á[1F]ä[1C]xók¹;CÆ[10]<b;%p±Û[10]b[07]Ô[04]Dz[07]48¨jÜ}$Ð[07]uí»[19][05]äwF[1E]Lbœ$ôr]aâ[05]ž	'±o4{h/_ëÑF~[01]]#[15]à±Z8×[07][14]hï£Lh:Œ¨¸¤øç†±F7#d|1€ã48çÞ[1F][1D]64~½%b[17]Ã[03]=
RSSI: -46
Sent a reply
got request: 0,123,456,789E[12].±¦[0F][08][07]1[0B][11]'QØ"d»%¡[0C]([01]¦)[18]æAM![17]†$¹4[08]Ù8œ;÷#£xÃ@—Šh6[0F]ýn"´%½M¹e;Á[1F]ä[1C]xók¹;CÆ[10]<b;%p±Û[10]b[07]Ô[04]Dz[07]48¨jÜ}$Ð[07]uí»[19][05]äwF[1E]Lbœ$ôr]aâ[05]ž	'±o4{h/_ëÑF~[01]]#[15]à±Z8×[07][14]hï£Lh:Œ¨¸¤øç†±F7#d|1€ã48çÞ[1F][1D]64~½%b[17]Ã[03]=
RSSI: -46
Sent a reply
got request: 0,123,456,789E[12].±¦[0F][08][07]1[0B][11]'QØ"d»%¡[0C]([01]¦)[18]æAM![17]†$¹4[08]Ù8œ;÷#£xÃ@—Šh6[0F]ýn"´%½M¹e;Á[1F]ä[1C]xók¹;CÆ[10]<b;%p±Û[10]b[07]Ô[04]Dz[07]48¨jÜ}$Ð[07]uí»[19][05]äwF[1E]Lbœ$ôr]aâ[05]ž	'±o4{h/_ëÑF~[01]]#[15]à±Z8×[07][14]hï£Lh:Œ¨¸¤øç†±F7#d|1€ã48çÞ[1F][1D]64~½%b[17]Ã[03]=


So the characters are randomly generated the first time, but once they are generated the first time, they do not change.

Any ideas?

TomWS

I suggest that, when you send, send sendlen+1 characters so the null termination character is sent as well.  You're sending unterminated character strings.

Tom

dave_sausages

is that as simple as changing
rf95.send((uint8_t*)buffer, sendLen);

to
rf95.send((uint8_t*)buffer, sendLen+1);

?

TomWS

#3
Quote from: dave_sausages on August 28, 2016, 08:37:30 PM
is that as simple as changing
rf95.send((uint8_t*)buffer, sendLen);

to
rf95.send((uint8_t*)buffer, sendLen+1);

?
Yup.

BTW, sprintf returns the length of the final string already.  Change your code from:
sprintf(buffer, "%d,%d,%d,%d", armed, dave, dave2, dave3);
    sendLen = strlen(buffer);  //get the length of buffer


to

sendLen = sprintf(buffer, "%d,%d,%d,%d", armed, dave, dave2, dave3);

dave_sausages

Thank you very much TomWS!  :) I did what you said and it worked. It seems weird that when I search for SendLen+1 on the entire moteino forum, only my post matches. I would have thought this would have been a fairly common problem. Or maybe it's because I'm using the unreliable client and server and most other use the reliable ones?

TomWS

Quote from: dave_sausages on August 28, 2016, 09:01:27 PM
Thank you very much TomWS!  :) I did what you said and it worked. It seems weird that when I search for SendLen+1 on the entire moteino forum, only my post matches. I would have thought this would have been a fairly common problem. Or maybe it's because I'm using the unreliable client and server and most other use the reliable ones?
Often packet length is determine by radio.DATALEN.  Since you were dealing with null terminated character arrays, it was natural for you to assume that the termination would be sent as well.  It's a very fundamental C error.  Glad you've solved your problem.

Tom

syrinxtech

#6
Something I've done from programming in C for over 30 years is to clear out all buffers before using them:

memset(&buffer, '\0', sizeof(buffer));     <- if buffer is a struct

memset(buffer, '\0', sizeof(buffer));       <- if buffer is a char * or char[]


I never have to do the "+1" trick.

The above basically sets all available space to the null character so even if you don't add one it's in there.