Important changes to forums and questions
All forums and questions are now archived. To start a new conversation or read the latest updates go to forums.mbed.com.
6 years, 5 months ago.
Very strange behaviour with Timer and UART interrupts - NUCLEO F091RC
Hello all,
I've come across a very frustrating intermittent fault in my program and cannot seem to solve the issue.
I'm using an STM32 NUCLEO F091RC microcontroller to receive GPS data over UART (using a serial interrupt) and then send it over LoRa (using a timer interrupt).
The weird issue that I am having: The program occasionally gets stuck in the Timer interrupt (just loops through it, without even entering main) seemingly when the GPS gets a fix. This happens around one time in every 5.
I suspect that it could be something to do with the UART interrupt taking slightly longer when getting a fix because of the longer NMEA sentence and the interaction with the timer interrupt causes an issue, but do not know for sure, or how to fix this!
The GPS code works perfectly fine on its own. As soon as I introduce the timer to send over LoRa the issue occurs. The code can also run fine without getting a GPS fix, but as soon as the extra parsing occurs, the UART interrupt stops firing and the program gets stuck in the timer interrupt loop. I know that the GPS is not the issue because I've connected it to an Arduino after the UART hang-up and it still transmits valid data over UART.
Please see the ISR code below. Any help would be greatly appreciated!
include the mbed library with this snippet
// #ifdef DEBUGGER
RawSerial pc(USBTX, USBRX); // USART2
// #endif
RawSerial gps(GPS_TX, GPS_RX); // USART1
DigitalOut GPS_status(GPS_STATUS_LED);
RMC_data RMC;
GPS_data GPS_parse;
volatile int Char_index = 0; // index for char array
char Rx_data[MAX_NMEA_LENGTH] = "0"; // char array to store received bytes
volatile uint8_t GPS_data_ready = 0;
volatile uint8_t sending = 0;
volatile bool timerset = false;
volatile bool firstfix = false;
volatile bool keepOut = false;
char UTC_Time[11];
Ticker LoRaSend_timer;
//void flushSerialBuffer(void) { char char1 = 0; while (gps.readable()) { char1 = gps.getc(); } return; }
void gps_receive(void) {
__disable_irq();
keepOut = true;
if (sending) //
{ //
Char_index = 0; //
memset(Rx_data,0,sizeof(Rx_data)); //
__enable_irq(); //
keepOut = false; //
return; //
} //*/
while(gps.readable()) {
char incoming = gps.getc();
Rx_data[Char_index] = incoming;
if((Rx_data[Char_index] == '\n')){ // Received the end of an NMEA scentence
if((strncmp(GPRMC_Tag,Rx_data,(sizeof(GPRMC_Tag)-1)) == 0) || (strncmp(GNRMC_Tag,Rx_data,(sizeof(GNRMC_Tag)-1)) == 0)){
RMC = Parse_RMC_sentence(Rx_data);
if(strcmp(RMC.Status, "A") == 0){
//GPS_status = 0;
}
else{
// GPS_status = 1;
}
GPS_status=!GPS_status;
GPS_data_ready = 0;
GPS_parse = Parse_RMC_data(RMC);
GPS_data_ready = 1;
}
Char_index = 0;
memset(Rx_data,0,sizeof(Rx_data));
}
else
Char_index++;
}
keepOut = false;
__enable_irq();
return;
}
void LoRaSend(){
if (keepOut)
return;
__disable_irq();//NVIC_DisableIRQ(USART1_IRQn);
if (strcmp(UTC_Time, GPS_parse.UTC_Time) == 0)
{ // use this to capture the UART interrupt hang-up
pc.putc('C');
gps.attach(NULL,Serial::RxIrq);
wait(1);
gps.attach(&gps_receive, Serial::RxIrq);
//NVIC_DisableIRQ(USART1_IRQn);
}
strcpy(UTC_Time,GPS_parse.UTC_Time);
if(GPS_data_ready == 1){
sending = 1;
char AppDatas[APPDATA_SIZE];
strcpy(AppDatas, GPS_parse.Latitude);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Longitude);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Course_Over_Ground);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Speed_Over_Ground);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.UTC_Time);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Valid);
McpsReq_t mcpsReq;
uint8_t AppPort = 3;
mcpsReq.Type = MCPS_UNCONFIRMED;
mcpsReq.Req.Unconfirmed.fPort = AppPort;
mcpsReq.Req.Unconfirmed.fBuffer = AppDatas;
mcpsReq.Req.Unconfirmed.fBufferSize = APPDATA_SIZE+2;
mcpsReq.Req.Unconfirmed.Datarate = DR_5;
GPS_status = !GPS_status;
LoRaMacMcpsRequest( &mcpsReq );
GPS_data_ready = 0;
}
sending = 0;
__enable_irq();//NVIC_EnableIRQ(USART1_IRQn);
pc.putc('N');
}
Below is the altered interrupt driven approach: Thanks for your answer! All very useful stuff to know and your proposed program structure makes a lot of sense. I've implemented your suggestions, but the issue appears to persist. I've posted my implementation below:
#include "mbed.h"
//#include "mbed_events.h"
#include "board.h"
#include "radio.h"
#include "LoRaMac.h"
#include "l86.hpp"
#define APPDATA_SIZE 45
#define LORAWAN_DEFAULT_DATARATE DR_0
#define LORAWAN_DEVICE_ADDRESS ( uint32_t )0x01234567
#define LORAWAN_NWKSKEY { 0x11, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01 }
#define LORAWAN_APPSKEY { 0x11, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01 }
char AppDatas[APPDATA_SIZE];
static uint8_t NwkSKey[] = LORAWAN_NWKSKEY;
static uint8_t AppSKey[] = LORAWAN_APPSKEY;
// #ifdef DEBUGGER
RawSerial pc(USBTX, USBRX); // USART2
// #endif
RawSerial gps(GPS_TX, GPS_RX); // USART1
DigitalOut GPS_status(GPS_STATUS_LED);
RMC_data RMC;
GPS_data GPS_parse;
volatile int Char_index = 0; // index for char array
char Rx_data[MAX_NMEA_LENGTH] = "0"; // char array to store received bytes
char Rx_data_copy[MAX_NMEA_LENGTH] = "0"; // char array to store received bytes
volatile bool sendLoRaNow = false;
volatile bool gpsReady = false;
volatile bool LoRaReady = false;
McpsReq_t mcpsReq;
Ticker LoRaSend_timer;
//void flushSerialBuffer(void) { char char1 = 0; while (gps.readable()) { char1 = gps.getc(); } return; }
void gps_receive(void) {
while(gps.readable() && !gpsReady) {
char incoming = gps.getc();
Rx_data[Char_index] = incoming;
if((Rx_data[Char_index] == '\n')){ // Received the end of an NMEA scentence
GPS_status = !GPS_status;
if((strncmp(GPRMC_Tag,Rx_data,(sizeof(GPRMC_Tag)-1)) == 0) || (strncmp(GNRMC_Tag,Rx_data,(sizeof(GNRMC_Tag)-1)) == 0)){
gpsReady = true;
//strcpy(Rx_data_copy, Rx_data);
Rx_data[Char_index+1] = 0;
}
Char_index = 0;
}
else
Char_index++;
}
}
void timerisr(){
if(LoRaReady)
sendLoRaNow = true;
LoRaReady = false;
pc.putc('t');
}
void createLoRaMessage(){
strcpy(AppDatas, GPS_parse.Latitude);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Longitude);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Course_Over_Ground);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Speed_Over_Ground);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.UTC_Time);
strcat(AppDatas, ",");
strcat(AppDatas, GPS_parse.Valid);
mcpsReq.Req.Unconfirmed.fBuffer = AppDatas;
}
void McpsConfirm( McpsConfirm_t *mcpsConfirm )
{
return;
}
void McpsIndication( McpsIndication_t *mcpsIndication )
{
return;
}
int main( void )
{
//Initialise firmware
LoRaMacPrimitives_t LoRaMacPrimitives;
LoRaMacCallback_t LoRaMacCallbacks;
MibRequestConfirm_t mibReq;
LoRaMacPrimitives.MacMcpsConfirm = McpsConfirm;
LoRaMacPrimitives.MacMcpsIndication = McpsIndication;
// LoRaMacPrimitives.MacMlmeConfirm = McpsConfirm;
LoRaMacInitialization( &LoRaMacPrimitives, &LoRaMacCallbacks );
LoRaMacChannelAdd( 3, ( ChannelParams_t ){ 868100000, { ( ( DR_5 << 4 ) | DR_5 ) }, 3 } );
//LoRaMacChannelAdd( 4, ( ChannelParams_t ){ 867300000, { ( ( DR_5 << 4 ) | DR_0 ) }, 0 } );
//LoRaMacChannelAdd( 5, ( ChannelParams_t ){ 867500000, { ( ( DR_5 << 4 ) | DR_0 ) }, 0 } );
//LoRaMacChannelAdd( 6, ( ChannelParams_t ){ 867700000, { ( ( DR_5 << 4 ) | DR_0 ) }, 0 } );
//LoRaMacChannelAdd( 7, ( ChannelParams_t ){ 867900000, { ( ( DR_5 << 4 ) | DR_0 ) }, 0 } );
//LoRaMacChannelAdd( 8, ( ChannelParams_t ){ 868800000, { ( ( DR_7 << 4 ) | DR_7 ) }, 2 } );
//LoRaMacChannelAdd( 9, ( ChannelParams_t ){ 868300000, { ( ( DR_6 << 4 ) | DR_6 ) }, 1 } );
//Join ABP
mibReq.Type = MIB_NET_ID;
mibReq.Param.NetID = 0;
LoRaMacMibSetRequestConfirm( &mibReq );
mibReq.Type = MIB_DEV_ADDR;
mibReq.Param.DevAddr = LORAWAN_DEVICE_ADDRESS;
LoRaMacMibSetRequestConfirm( &mibReq );
mibReq.Type = MIB_NWK_SKEY;
mibReq.Param.NwkSKey = NwkSKey;
LoRaMacMibSetRequestConfirm( &mibReq );
mibReq.Type = MIB_APP_SKEY;
mibReq.Param.AppSKey = AppSKey;
LoRaMacMibSetRequestConfirm( &mibReq );
mibReq.Type = MIB_NETWORK_JOINED;
mibReq.Param.IsNetworkJoined = true;
LoRaMacMibSetRequestConfirm( &mibReq );
mibReq.Type = MIB_CHANNELS_TX_POWER;
mibReq.Param.ChannelsTxPower = LORAMAC_MAX_TX_POWER;
LoRaMacMibSetRequestConfirm( &mibReq );
mibReq.Type = MIB_CHANNELS_DEFAULT_TX_POWER;
mibReq.Param.ChannelsDefaultTxPower = LORAMAC_MAX_TX_POWER;
LoRaMacMibSetRequestConfirm( &mibReq );
uint8_t AppPort = 3;
mcpsReq.Type = MCPS_UNCONFIRMED;
mcpsReq.Req.Unconfirmed.fPort = AppPort;
mcpsReq.Req.Unconfirmed.fBufferSize = APPDATA_SIZE+2;
mcpsReq.Req.Unconfirmed.Datarate = DR_5;
GPS_status = 1;
//gps.attach(&gps_receive, Serial::RxIrq);
gps.attach(&gps_receive, Serial::RxIrq);
gps.printf(PMTK_SET_NMEA_OUTPUT_RMC); // Only output RMC and GPTXT
wait(1.0);
gps.printf(PMTK_SET_UPDATE_F_2HZ); // Set update Frequency to 2Hz
wait(1.0);
gps.printf(PMTK_SET_NMEA_OUTPUT_RMC); // Only output RMC and GPTXT
wait(1.0);
gps.printf(PMTK_SET_UPDATE_F_2HZ); // Set update Frequency to 2Hz
wait(1.0);
NVIC_SetPriority(USART1_IRQn, 0);
LoRaSend_timer.attach(&timerisr, 1.0);
while(true){
if(gpsReady){
__disable_irq();
strcpy(Rx_data_copy, Rx_data);
gpsReady = false;
__enable_irq();
RMC = Parse_RMC_sentence(Rx_data_copy);
GPS_parse = Parse_RMC_data(RMC);
createLoRaMessage();
LoRaReady = true;
Print_RMC_data(&RMC);
//Print_GPS_data(GPS_parse);
}
if(sendLoRaNow){
sendLoRaNow = false;
LoRaMacMcpsRequest( &mcpsReq );
}
}
}
Thanks for the help!
3 Answers
6 years, 5 months ago.
Can you please post only the interrupt handler code?
Without diving in too much, it looks like youre doing A LOT in your ISR. It is best not to perform any blocking operations in an ISR.
Please see this blog post about what's bad about long operations in an ISR - https://os.mbed.com/blog/entry/Simplify-your-code-with-mbed-events/
Please see this tutorial for more information of the eventQueue - https://os.mbed.com/docs/v5.6/tutorials/the-eventqueue-api.html
posted by 06 Dec 2017Thanks for your answer Sarah! I've been looking into using this event driven approach. Unfortunately I'm having issues with importiong the mbed-os library. I keep getting compilation errors with the LoRa library when the mbed-os library is in the same directory is in the program directory
posted by 08 Dec 2017Hi Sarah, I've implemented a solution using eventQueue as suggested. I now do all the work outside the context of the ISR and simply use the ISR to call an event. I have one event for the timer and one for the UART interrupt. I'm still getting the same issue :/. Would you like to see the eventQueue code?
posted by 11 Dec 2017
6 years, 5 months ago.
A couple of simple things...
Firstly unless you are playing with interrupt priorities somewhere else the timer and uart are the same priority. This means there is no need to disable interrupts or set flags to avoid them interrupting each other, the only thing that can interrupt an ISR is one with a higher priority setting.
Secondly in your uart ISR you can avoid the memset command and save a bit of time. Just add one line so that the code is
if((Rx_data[Char_index] == '\n')) { // Received the end of an NMEA scentence
Rx_data[Char_index+1] = 0;
And you will null terminate the string without having to blank the entire buffer every time. If your NMEA parser doesn't care about the \n on the end then you can use Char_index rather than Char_index+1 and simply replace the \n with a null.
Finally you are using a lot of string manipulation function calls. While these are very convenient they aren't always the fastest way to do things. e.g. you do two comparisons for GPRMC and GNRMC, instead check the second character and if it's a P change it to an N and then do a single comparison. Or even better just ignore the GP/GN part and check that it has RMC in the correct place. And all of those strcats, each time it has to search the current string to find the end in order to add the new string. It's effectively the same as doing strcpy(AppDatas+strlen(AppDatas),TextToAdd). If you know the length of the string so far and the lengths of the fields you are adding then you can avoid the strlen part and track the length of the string manually. Yes, these are all fairly minor things and individually they aren't going to make much difference but inside an interrupt the trade off between readability and efficiency needs to be more towards efficiency than in normal background code.
But really the best solution would be to change the structure to something like:
volatile bool gpsWaiting = false;
bool LoranReady = false;
volatile bool sendLoranNow = false;
void uart_isr() {
while (!gpsWaiting && gps.readable()) {
Rx_data[Char_index] = gps.getc();
if ( Rx_data[Char_index] == '\n') {
Rx_data[Char_index] = 0;
gpsWaiting = true;
}
}
}
void timerisr() {
if(loranReady)
sendLoranNow = true;
loranReady = false;
}
main () {
// setup....
while (true) {
if (gpsReady) {
__disable_irq();
strcpy(Rx_data,RxData_Copy);
Char_index = 0;
gpsReady = false;
__enable_irq();
parse(RxData_Copy);
createLoranMessage();
loranReady = true;
}
if (sendLoranNow) {
sendLoranNow = false;
LoRaMacMcpsRequest( &mcpsReq );
}
}
}
Clearly this isn't complete code but it should give you an idea for the structure. All the serial port interrupt does is put the data into a buffer and set a flag when it gets the end of a line. All the timer interrupt does is set a flag to indicate that any waiting data should be sent.
The main loop then does the bulk of the work in the background, it disables the interrupts while it copies the serial buffer and then re-enables them so that a new message can start to arrive in the background while this one is being processed. Rather than generating the loran message during the timer interrupt instead create it in the background as soon as the gps data is received. Depending on timer / gps data rates this may result in messages being created and then discarded without being send and so a slightly higher average CPU load but that work is in a non time critical part of the code rather than something time critical. Whenever a radio message is created we then set a flag indicating that there is a new message ready to send. The timer then sets a flag indicating that the message should be sent now at which point the background loop then does the actual transmission. Yes, this will add a slight delay to the transmission time but less of a delay than generating the whole message when the timer goes off. And yes transmission will be delayed if we are processing a serial message when the timer fires but with your current code that is also the case so it's no significant difference either way.
5 years, 7 months ago.
Hi Rishin Amin, You sort out above problem(GPS data transmission with lora )? if you sort out please help me? I am also facing same issue to transmit GPS data through sx1272 lora module. in the program control stucks in wait longer time and no data transmission through lora . it is working fine with out lora stuff. please help me how to resolve this issue? Thanks in advance. Regards Ashok