Is this a proper implementation of FreeRTOS for Arduino?

Question

I'm using the RGBLCDShield library and a few analog input signals and since it seems the interrupts are being used by the SPI interface of the LCD Shield, I decided to try it with FreeRTOS instead. I don't currently have my arduino with me (left if at parents') but I wanted to get some feedback on this code to see if it would run properly; I might not be getting my UNO back for a few days but I wanted to at least have a usable code ready to be tested when I do get it.

The Analog inputs are just 2 low level signals and the digital outputs just control some IC switches. One PWM is fed through a lowpass filter and into an amplifier input.

Here is my code in it's entirety:

#include <Arduino_FreeRTOS.h>
#include <Adafruit_RGBLCDShield.h>
#include <Wire.h>
#include <math.h>
// define tasks
void TaskAnalogRead( void *pvParameters ); // Check voltage input
void TaskLCDShield( void *pvParameters ); // Control Display
void TaskUserInput( void *pvParameters ); // User Interface - System Process
Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();
unsigned int vSet = 0; // Desired Voltage (potentiometer set)
unsigned int vOut = 0; // Voltage on capacitor
// the setup function runs once when you press reset or power the board
void setup() {
 // Set up Tasks to run independently
 xTaskCreate(
 TaskAnalogRead
 , (const portCHAR *) "AnalogRead" // A name just for humans
 , 128 // This stack size can be checked & adjusted by reading the Stack Highwater
 , NULL
 , 2 // Priority, with 1 being the highest, and 4 being the lowest.
 , NULL );
 xTaskCreate(
 TaskLCDShield
 , (const portCHAR *) "LCDShield"
 , 128 // Stack size
 , NULL
 , 3 // Priority
 , NULL );
 xTaskCreate(
 TaskUserInput
 , (const portCHAR *) "UserInput"
 , 128
 , NULL
 , 1
 , NULL );
 // Now the task scheduler, which takes over control of scheduling individual tasks, is automatically started.
 lcd.begin(16, 2);
}
void loop()
{
 // Empty. Things are done in Tasks.
}
/*--------------------------------------------------*/
/*---------------------- Tasks ---------------------*/
/*--------------------------------------------------*/
void TaskLCDShield(void *pvParameters) // This is a task.
{
 (void) pvParameters;
 // Start Display
 lcd.setBacklight(0x7);
 lcd.setCursor(0,0);
 lcd.println(" PORTABLE SPARK ");
 delay(5000);
 lcd.clear();
 for (;;) // Continuously Update display
 {
 int volts = vSet * 100.0;
 int chg = (vOut * 0.488)/volts;
 int vpos, cpos;
 lcd.setCursor(0,0);
 lcd.print("Voltage: ");
 lcd.setCursor(15, 0);
 lcd.print("V");
 lcd.setCursor(0,1);
 lcd.print("Charge : ");
 lcd.setCursor(15,1);
 lcd.print("%");
 lcd.setCursor(9,0);
 if(volts < 100)
 {
 lcd.print(" ");
 vpos = 13;
 }
 else if(volts < 1000)
 {
 lcd.print(" ");
 vpos = 11;
 }
 else if(volts < 10000)
 {
 lcd.print(" ");
 vpos = 10;
 }
 else vpos = 9;
 lcd.setCursor(vpos,0);
 lcd.print(volts, DEC);
 lcd.setCursor(9,1);
 if(chg < 10)
 {
 lcd.print(" ");
 cpos = 13;
 }
 else if(chg < 100)
 {
 lcd.print(" ");
 cpos = 12;
 }
 else cpos = 11;
 lcd.setCursor(cpos,1);
 lcd.print(chg, DEC);
 //digitalWrite(13, HIGH); // turn the LED on (HIGH is the voltage level)
 vTaskDelay( 1000 / portTICK_PERIOD_MS ); // wait for one second
 //digitalWrite(13, LOW); // turn the LED off by making the voltage LOW
 //vTaskDelay( 1000 / portTICK_PERIOD_MS ); // wait for one second
 }
}
void TaskAnalogRead(void *pvParameters) // Read Analog Voltages (VSet, Emonitor)
{
 (void) pvParameters;
 for (;;)
 {
 int eMon = analogRead(A0); // read input pins
 vTaskDelay(1);
 int vA = analogRead(A1);
 vSet = (vA/1024) * 15000; // calculate actual values
 vOut = (5.0/1024.0) * 10000 * eMon; 
 vTaskDelay(10); // one tick delay (300ms) in between reads
 }
} // End Analog Read
void TaskUserInput(void *pvParameters) // User Interface
{
 (void) pvParameters;
 // initialize digital pin 13 as an output.
 pinMode(8, OUTPUT); // Fire signal
 pinMode(7, OUTPUT); // Charge/Discharge signal
 pinMode(5, OUTPUT); // Eprgm PWM output
 pinMode(4, OUTPUT); // Cam/DAQ trigger
 bool charge = false; // charge flag
 for (;;)
 {
 uint8_t button = lcd.readButtons();
 switch(charge)
 {
 case true:
 analogWrite(5, (vSet * 0.0204)/5);
 vTaskDelay(1);
 digitalWrite(7, HIGH);
 vTaskDelay(1);
 break;
 default:
 analogWrite(5, 0);
 vTaskDelay(1);
 digitalWrite(7, LOW);
 vTaskDelay(1);
 break;
 } // End Switch(charge)
 if(button & BUTTON_UP) // Charge/Discharge button
 {
 charge = !charge;
 } // end if
 else if(button & BUTTON_DOWN) // Fire!
 {
 digitalWrite(8, HIGH);
 vTaskDelay(1);
 digitalWrite(4, HIGH);
 vTaskDelay(1);
 digitalWrite(7, LOW);
 charge = false;
 vTaskDelay(1);
 digitalWrite(4, LOW);
 vTaskDelay(1);
 digitalWrite(8, LOW);
 } // End else if
 } // End for
} // End User Interface

My specific concern is with the 2wire interface of the LCD shield and whether it would be able to communicate with the Arduino - especially when it needs to read the button inputs.
Also, how do I determine the stack size for each task? Is 128 for each enough to handle what I have?

• Edgar Bonet: "the interrupts are being used by the SPI interface of the LCD Shield". I don't quite understand this sentence. Each interrupt source has its own interrupt (or interrupts). The SPI interface has one interrupt. It does not prevent you from using other interrupts if you want.

Answer 1

This is not a direct answer to the question. Instead, I suggest that you do not need an RTOS to do what you want. A lightweight alternative is to use millis() to decide when it is time to perform such or such task:

void loop()
{
 uint32_t now = millis();
 // Update the LCD every second.
 static uint32_t last_lcd_update;
 if (now - last_lcd_update >= 1000) {
 last_lcd_update = now;
 update_lcd();
 }
 // Read the analog inputs every 300 ms.
 static uint32_t last_analog_read;
 if (now - last_analog_read >= 300) {
 last_analog_read = now;
 do_analog_read();
 }
 // User input is handled continuously.
 handle_user_input();
}

This will use significantly less resources (flash, RAM and CPU time) than an RTOS. Another advantage is that you do not need to determine a stack size for each task, which is definitely not a simple thing to do.