TheThingsNetwork / arduino-device-lib

Arduino Library for TTN Devices
MIT License
208 stars 96 forks source link

ttn.join hangs #172

Closed 0xceb1d closed 7 years ago

0xceb1d commented 7 years ago

Having issues with the ttn.join command, ttn.status responds, however when a join is attempted the Arduino/IDE seems to hang (nothing prints. have to unplug board from machine to be able to close serial monitor).

Tested with both a custom dev board (SAMD21) and also Arduino Due using TX/RX1, reset pin flipped before calling ttn.status. Using example code from library (Send).

0xceb1d commented 7 years ago

Additional info:

FokkeZB commented 7 years ago

Do you have an example sketch for us to reproduce? Did you try using the 4th retry argument to limit the number of times it retries? See https://github.com/TheThingsNetwork/arduino-device-lib/blob/master/src/TheThingsNetwork.h#L89

0xceb1d commented 7 years ago

appKey ommited:

#include <TheThingsNetwork.h>
#include <TheThingsMessage.h>

// Set your AppEUI and AppKey
const char *appEui = "70B3D57EF0001BEF";
const char *appKey = "";

#define loraSerial Serial2
#define debugSerial SerialUSB

// Replace REPLACE_ME with TTN_FP_EU868 or TTN_FP_US915
#define freqPlan TTN_FP_EU868

TheThingsNetwork ttn(loraSerial, debugSerial, freqPlan);

devicedata_t data = api_DeviceData_init_default;

void setup() {
  loraSerial.begin(57600);
  debugSerial.begin(57600);

  while (!debugSerial && millis() < 10000);

  pinMode(17, OUTPUT);
  pinMode(28, OUTPUT);

  digitalWrite(17, LOW);  
  delay(1000);             
  digitalWrite(17, HIGH);  
  delay(1000);

  // Wait a maximum of 10s for Serial Monitor

  while (!loraSerial && millis() < 10000);

  debugSerial.println("-- STATUS");
  ttn.showStatus();

  delay(3000);

  debugSerial.println("-- JOIN");
  ttn.join(appEui, appKey, 2);

  digitalWrite(28, HIGH);    // turn the LED off by making the voltage LOW

  // Select what fields to include in the encoded message
  data.has_motion = true;
  data.has_water = false;
  data.has_temperature_celcius = true;
  data.has_temperature_fahrenheit = true;
  data.has_humidity = true;
}

void loop() {
  debugSerial.println("-- LOOP");

  // Read the sensors
  data.motion = true;
  data.water = 682;
  data.temperature_celcius = 30;
  data.temperature_fahrenheit = 86;
  data.humidity = 97;

  // Encode the selected fields of the struct as bytes
  byte *buffer;
  size_t size;
  TheThingsMessage::encodeDeviceData(&data, &buffer, &size);

  ttn.sendBytes(buffer, size);

  delay(10000);
}

Output before hang:

-- STATUS
EUI: 0004A30B001C4D8F
Battery: 3273
AppEui: 0000000000000000
DevEui: 0004A30B001C4D8F
Data Rate: 5
RX Delay 1: 1000
RX Delay 2: 2000
Total Airtime: 0.00 s
-- JOIN
0xceb1d commented 7 years ago

It is pretty much based on the Send example, just adding in a reset for the RN2483 and using Pin 28 to indicate status

FokkeZB commented 7 years ago

Thanks @Oliverr. I don't see a reset in your example. Why would you need that?

I would always suggest to first try Send.ino and even remove all accept the first line of the loop() there to just rule out any other causes.

0xceb1d commented 7 years ago

Hardware reset:

digitalWrite(17, LOW);  
delay(1000);             
digitalWrite(17, HIGH);  
delay(1000);

I'll try the send.ino and report back

0xceb1d commented 7 years ago
#include <TheThingsNetwork.h>

// Set your AppEUI and AppKey
const char *appEui = "70B3D57EF0001BEF";
const char *appKey = "OMITTED";

#define loraSerial Serial2
#define debugSerial Serial

// Replace REPLACE_ME with TTN_FP_EU868 or TTN_FP_US915
#define freqPlan TTN_FP_EU868

TheThingsNetwork ttn(loraSerial, debugSerial, freqPlan);

void setup() {
  loraSerial.begin(57600);
  debugSerial.begin(57600);

  // Wait a maximum of 10s for Serial Monitor
  while (!debugSerial && millis() < 10000);

  debugSerial.println("-- STATUS");
  ttn.showStatus();

  debugSerial.println("-- JOIN");
  ttn.join(appEui, appKey);
}

void loop() {
  debugSerial.println("-- LOOP");

  // Prepare payload of 1 byte to indicate LED status
  //byte payload[1];
  //payload[0] = (digitalRead(LED_BUILTIN) == HIGH) ? 1 : 0;

  // Send it off
  //ttn.sendBytes(payload, sizeof(payload));

  delay(10000);
}

Produces: -- STATUS

FokkeZB commented 7 years ago

Looking at your other sketch Serial should be SerialUSB right?

0xceb1d commented 7 years ago

Correct, Serialis an alias of SerialUSB

mikimite commented 7 years ago

Hi, I tried to use it in an AVR, so i found the microcontroller resets itself at a certain moment. I found that the code fails returning from this funtion: void TheThingsNetwork::configureEU868(uint8_t sf) Hope it helps

mikimite commented 7 years ago

found it: changechar dr[1]; per char dr[2];

FokkeZB commented 7 years ago

You mean at https://github.com/TheThingsNetwork/arduino-device-lib/blob/master/src/TheThingsNetwork.cpp#L614 and https://github.com/TheThingsNetwork/arduino-device-lib/blob/master/src/TheThingsNetwork.cpp#L655?

FokkeZB commented 7 years ago

@Nicolasdejean please verify

mikimite commented 7 years ago

I wasn't at the last revision. the lines are already changed in the last revision the fix:https://github.com/TheThingsNetwork/arduino-device-lib/pull/157 the lines https://github.com/TheThingsNetwork/arduino-device-lib/blob/master/src/TheThingsNetwork.cpp#L564 & https://github.com/TheThingsNetwork/arduino-device-lib/blob/master/src/TheThingsNetwork.cpp#L622

FokkeZB commented 7 years ago

Ah, @Oliverr is this the same issue you had? Make sure you are on version 2.0 of the library

0xceb1d commented 7 years ago

@FokkeZB I can confirm I am using 2.0.0, still getting locked up during .join()

0xceb1d commented 7 years ago

Just checked TheThingsNetwork.cpp, showing char dr[2]; on lines 564 and 622

FokkeZB commented 7 years ago

OK, so @mikimite had a different issue then.

@Oliverr what I find weird about your findings with https://github.com/TheThingsNetwork/arduino-device-lib/issues/172#issuecomment-268542507 is that it got stuck at showStatus() and not join() while with https://github.com/TheThingsNetwork/arduino-device-lib/issues/172#issuecomment-268530795 it was indeed join(). The only difference I see is the use of SerialUSB instead of Serial and the digitalWrite() to pin 17. Any idea?

0xceb1d commented 7 years ago

Away at the moment, let me check it in a couple of days. I will make sure the 2 sketches are setup the same way (resets etc.)

0xceb1d commented 7 years ago

Just got back. Done some tidying and have the following scenarios:

#include <TheThingsNetwork.h>

#define loraSerial Serial2
#define debugSerial SerialUSB

// Replace REPLACE_ME with TTN_FP_EU868 or TTN_FP_US915
#define freqPlan TTN_FP_EU868

TheThingsNetwork ttn(loraSerial, debugSerial, freqPlan);

void setup()
{
  loraSerial.begin(57600);
  debugSerial.begin(57600);

  pinMode(17, OUTPUT);
  digitalWrite(17, LOW);  
  delay(1000);             
  digitalWrite(17, HIGH);  
  delay(500);

}

void loop()
{
  debugSerial.println("Device Information");
  debugSerial.println();
  ttn.showStatus();
  debugSerial.println();
  debugSerial.println("Use the EUI to register the device for OTAA");
  debugSerial.println("-------------------------------------------");
  debugSerial.println();

  delay(10000);
}

This works fine:

Device Information

EUI: 0004A30B001C4D8F
Battery: 3263
AppEui: 70B3D57EF0001BEF
DevEui: 0004A30B001C4393
Data Rate: 5
RX Delay 1: 1000
RX Delay 2: 2000
Total Airtime: 0.00 s

Use the EUI to register the device for OTAA
-------------------------------------------

Moving onto the Send scenario, when running this Send example:

#include <TheThingsNetwork.h>

// Set your AppEUI and AppKey
const char *appEui = "70B3D57EF0001BEF";
const char *appKey = "OMITTED";

#define loraSerial Serial2
#define debugSerial SerialUSB

// Replace REPLACE_ME with TTN_FP_EU868 or TTN_FP_US915
#define freqPlan TTN_FP_EU868

TheThingsNetwork ttn(loraSerial, debugSerial, freqPlan);

void setup() {
  loraSerial.begin(57600);
  debugSerial.begin(57600);

  // Wait a maximum of 10s for Serial Monitor
  //while (!debugSerial && millis() < 10000);

  pinMode(17, OUTPUT);
  digitalWrite(17, LOW);  
  delay(1000);             
  digitalWrite(17, HIGH);  
  delay(500);

  debugSerial.println("-- STATUS");
  ttn.showStatus();

  debugSerial.println("-- JOIN");
  ttn.join(appEui, appKey);
}

void loop() {
  debugSerial.println("-- LOOP");

  // Prepare payload of 1 byte to indicate LED status
  byte payload[1];
  payload[0] = (digitalRead(LED_BUILTIN) == HIGH) ? 1 : 0;

  // Send it off
  ttn.sendBytes(payload, sizeof(payload));

  delay(10000);
}

This hangs at the .join() stage, locking up the serial monitor:

-- STATUS
EUI: 0004A30B001C4D8F
Battery: 3263
AppEui: 70B3D57EF0001BEF
DevEui: 0004A30B001C4393
Data Rate: 5
RX Delay 1: 1000
RX Delay 2: 2000
Total Airtime: 0.00 s
-- JOIN
0xceb1d commented 7 years ago

I have also tested the hardware and gateway configuration with the following code successfully:

#define lora Serial2
#define debugSerial SerialUSB

int tempPin = A9;

void sendCmd( char *cmd) {
  Serial.write( cmd );
  Serial.write("\n");
  lora.write(cmd);
  lora.write("\r\n");
  while (!lora.available() ) {
    delay(100);
  }
  while (lora.available())
    Serial.write(lora.read());
}

void waitForResponse() {
  while (!lora.available() ) {
    delay(100);
  }
  while (lora.available())
    Serial.write(lora.read());
}

char getHexHi( char ch ) {
  char nibble = ch >> 4;
  return (nibble > 9) ? nibble + 'A' - 10 : nibble + '0';
}
char getHexLo( char ch ) {
  char nibble = ch & 0x0f;
  return (nibble > 9) ? nibble + 'A' - 10 : nibble + '0';
}

void sendData( char *data) {
  Serial.write( "mac tx uncnf 1 " );
  lora.write( "mac tx uncnf 1 " );
  // Write data as hex characters
  char *ptr = data;
  int idiotCount = 50;
  while (*ptr && idiotCount ) {
    lora.write( getHexHi( *ptr ) );
    lora.write( getHexLo( *ptr ) );

    Serial.write( getHexHi( *ptr ) );
    Serial.write( getHexLo( *ptr ) );

    ptr++;
    idiotCount--;
  }
  lora.write("\r\n");
  Serial.write("\n");
  delay(5000);

  while (lora.available())
    Serial.write(lora.read());
}

void setup()
{
  Serial.begin(57600);
  lora.begin(57600);

  while (!Serial && millis() < 10000);

  Serial.println("RN2483 Test");

  pinMode(17, OUTPUT);
  digitalWrite(17, LOW);
  delay(500);
  digitalWrite(17, HIGH);
  delay(500);

  waitForResponse();

  sendCmd("sys factoryRESET");

  sendCmd("mac set appkey OMITTED");
  sendCmd("mac set appeui 70B3D57EF0001BEF");
  sendCmd("mac save");
  sendCmd("mac join otaa");
  sendCmd("mac get status");
  sendCmd("mac get devaddr");

}

void loop() {
  // put your main code here, to run repeatedly:
  float temperature = 0.0;

  int tempReading = analogRead(tempPin);
  float voltage = tempReading * 3.2;
  voltage /= 1024.0; 
  float temperatureC = (voltage - 0.5) * 100 ;
  temperature = temperatureC;

  Serial.println(temperature);
  char msgBuf[40];
  sprintf(msgBuf, "{\"temp\":%d}", temperature);
  Serial.println(msgBuf);

  sendData(msgBuf);
  delay(5000);
  while (lora.available())
    Serial.write(lora.read());

  delay(60000);
}
FokkeZB commented 7 years ago

Thanks @Oliverr, @Nicolasdejean and @johanstokking will follow up with you.

johanstokking commented 7 years ago

@Oliverr do you mean that with the sketch you posted last, you do not get lock ups, but when using the library, you do?

0xceb1d commented 7 years ago

Correct, no issues with join and data is sent and is accessible in production (without the library).

johanstokking commented 7 years ago

Seems like we do some weird things in readLine().

It's hard to debug it remotely because I don't have your hardware setup here. Can you add some debugPrintLn() in the join() and provision() in TheThingsNetwork.cpp so we can at least see where the lock up starts?

johanstokking commented 7 years ago

@Oliverr OK I can confirm that we're screwing up with pointers, see #180, and it's typically device dependent how problems show.

0xceb1d commented 7 years ago

@johanstokking thanks for picking that up. Let me know if you need anything confirming