1
\$\begingroup\$

I have a PanStamp (arduino compatible) and everything was working fine until I change the amount of data I'm sending through serial.

send_altitude is called every second using timer interruption (TimerOne library).

This is the original function that sends data (and does not hang):

void send_altitude()
{
cli();

float_byte altitude;
byte buffer[6];

// Do one trash read to avoid fluctuations
analogRead(ALT_SENSOR);
delay(5);
altitude.asFloat = getAltitude(ALT_SENSOR);

buffer[0] = 0x05; /* Size */
buffer[1] = HUB_DATA;
buffer[2] = altitude.asByte[0];
buffer[3] = altitude.asByte[1];
buffer[4] = altitude.asByte[2];
buffer[5] = altitude.asByte[3];

send_through_serial(6, buffer);

sei();
}

But when I changed it to this one (basically I'm now sending a packet count): Following PeterJ suggestion I moved the buffer and variable outside the function and also removed the sei()/cli(), however it didn't work... One thing that I noticed is that it looks to take longer to hang without sei/cli.

float_byte altitude;
unsigned char buffer[10];
void send_altitude()
{
// Increments the counter;
packet_count.asULong = packet_count.asULong + 1;

// Do one trash read to avoid fluctuations
analogRead(ALT_SENSOR);
delay(5);
altitude.asFloat = getAltitude(ALT_SENSOR);

buffer[0] = 0x09; /* Size */
buffer[1] = HUB_DATA;
buffer[2] = altitude.asByte[0];
buffer[3] = altitude.asByte[1];
buffer[4] = altitude.asByte[2];
buffer[5] = altitude.asByte[3];

buffer[6] = packet_count.asByte[0];
buffer[7] = packet_count.asByte[1];
buffer[8] = packet_count.asByte[2];
buffer[9] = packet_count.asByte[3];

send_through_serial(10, buffer);
}

The PanStamp started to hang without reason.... packet_count is an ulong_byte.

The others functions and data structs:

void send_through_serial(int lenght, unsigned char *data)
{
unsigned int i;

/* Send srync word */
Serial.write(0xBA);
Serial.write(0xBA);

/* Send data */
for(i = 0; i < lenght; i++)
{
    Serial.write(data[i]);
}
}

/**
 * @brief getAltitude
 *
 * Return the altitude (Voltage) of a
 * analogue pressure sensor.
 *
 * Maybe later on it will return the
 * altitude
 *
 * @param ain Analogue pin
 * @return Altitude (Voltage)
 */
inline float getAltitude(int ain)
{
    int aRead = analogRead(ain);
    return aRead*(3.3/1023.0);
}


typedef union _float_byte
{
    byte asByte[4];
    float asFloat;
} float_byte;

typedef union _ulong_byte
{
    unsigned long asULong;
    byte asByte[4];
} ulong_byte;

Timer initialization code:

Timer1.initialize(1000000);
Timer1.attachInterrupt(send_altitude);

The only "solution" I found is use the WDT to reset after a hang, but it's not good because I have some counters that can not be rested.

Thanks to @JRobert it is working now!! I also made a few more changes:

  • Timer interrupt only setting a boolean flag;
  • Buffers inside the function;
  • Using sei/cli;
  • getAltitude() is not inline any more.
\$\endgroup\$
  • \$\begingroup\$ I can't see any problem above but how do you call send_altitude? Just thinking you may have a problem elsewhere and the additional delay of sending a few extra characters causes a problem. Maybe try doubling the baud rate if possible and see if that makes a difference. \$\endgroup\$ – PeterJ Aug 2 '13 at 10:12
  • \$\begingroup\$ I'm calling it every second using the TimerOne library (playground.arduino.cc/code/timer1). So I don't think that the problem is with the timing... I'll add this part of the code to the post \$\endgroup\$ – Tiago Queiroz Aug 2 '13 at 10:45
  • \$\begingroup\$ Try removing the cli / sei from your routine. Another thing to try is moving the buffer outside send_altitude so it's not on the stack. The extra space and the possibility of ending up with another nested interrupt being called might be overflowing the stack. \$\endgroup\$ – PeterJ Aug 2 '13 at 11:22
  • \$\begingroup\$ PeterJ, it didn't work... I tried with and without the sei/cli.... \$\endgroup\$ – Tiago Queiroz Aug 2 '13 at 14:47
  • \$\begingroup\$ Good work. I'm glad you fixed it. \$\endgroup\$ – JRobert Aug 6 '13 at 1:06
1
\$\begingroup\$

If nothing but the packet size changed, then the packet size is somehow related.

  • What is the baud rate?
  • Is there any reason one iteration of your code take anywhere near 1 sec to run?
  • How long can getAltitude() take to execute? Does it wait for hardware?

There's an awful lot of code executing at interrupt level - getAltitude(), and send_through_serial() (which also uses interrupt driven I/O). The first change I'd make would be to reduce the Timer1 interrupt code to setting a flag bit. Lift the rest of the code out of the interrupt routine into the main line. Call it whenever the flag bit gets set, and clear the flag bit. The worst that should happen by doing it this way is you could miss some interrupts if your main line code runs too long. Not that missing interrupts is acceptable; but it should stop the crashing and let you test your code further to find out what's being slow.

As to stack collisions, making a local variable into a static one won't reduce that likelihood; rather it could increase it. Locals get allocated and de-allocated, so there are times when they don't exist and can't contribute to a crash. Statics always exist and forever reduce the amount of memory for the stack can grow into. What might change though, is the behavior of the program when the collision occurs. When you make something static, it will be assigned a different location in memory; as a result, different variables might be involved when the collision occurs.

\$\endgroup\$
  • \$\begingroup\$ thanks a lot! Tthe baud rate is 9600 (Arduino's default), nothing on my code can take near 1 second. getAltitude() in only two lines: an analogue read and a float operation (I'll put the code on my post). It really looks like the only thing that makes the arduino to hang is the packet size, I changed it to 9 bytes and worked well (or I didn't waited enough). I'll do the changes that you suggested and on Monday, run some tests and post the results here. \$\endgroup\$ – Tiago Queiroz Aug 2 '13 at 20:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.