Function exiting for unknown reason

Hey all,
I recently posted about a class to remote control projectors. I am still working on the project, but am really stuck. I am trying to execute a function "start_sim" on button press, but as soon as this function calls a class function called "proj_pwr_on", the arduino (mega 2560) seems to crash, serial connection writes only the first two characters of the serial println in the function ("pw" is received as last message on Serial connection), then serial closes and nothing else happens. I am lost what is happening here, if I am running out of memory with the many Serial connections or anything else. Tried commenting my timer stuff etc, no chance. What am I missing?

/*
    Name:       arduino_pctl_v3.ino
    Created:	18/01/2024 15:40:22
    Author:     xxx
*/
#include "Button.h"
#include "Projector_wrapper.h"

//PORTA B00001000
#define pba_on_reg B00001000
#define pba_on_pin 25

//PORTA B00010000
#define pba_off_reg B00010000
#define pba_off_pin 26

//PORTA B00000001
#define pba_on_led1_reg B00000001
#define pba_on_led1 22

//PORTA B00000010
#define pba_on_led2_reg B00000010
#define pba_on_led2 23

//PORTA B00100000
#define pba_off_led1_reg B00100000
#define pba_off_led1 27

//PORTA B00000100
#define pba_off_led2_reg B00000100
#define pba_off_led2 24
//16e6 * (750 / 1000) / 256;
#define timer_blink_update_slow 46875
//16e6 * (250 / 1000) / 256;
#define timer_blink_update_fast 15625

#define number_of_proj 1

#define main_pwr 29
#define main_rst 28
#define main_feedback 30

unsigned int cooldown_time = 5000;
Button b_on;
Button b_off;
bool pba_on = false;
bool pba_off = false;
bool failure = false;
bool debug = false;

Projector_wrapper* ser_con[number_of_proj];
Projector_wrapper proj_1(1);
Projector_wrapper proj_2(2);
Projector_wrapper proj_3(3);

//state: 0 offline, 1 shutting down, 2 starting up, 3 ready without projectors, 4 fully ready, 5 failure
volatile uint8_t sim_state = 0;

ISR(TIMER1_COMPA_vect) {
    /*Serial.print("sim_state: ");
    Serial.println(sim_state);*/
    switch (sim_state) {
    case 0:
        PORTA &= ~pba_on_led1_reg;
        PORTA |= pba_off_led1_reg;
        break;
    case 1:
        PORTA &= ~pba_on_led1_reg;
        PORTA ^= pba_off_led1_reg;
        OCR1A += timer_blink_update_slow;
        break;
    case 2:
        PORTA &= ~pba_off_led1_reg;
        PORTA ^= pba_on_led1_reg;
        OCR1A += timer_blink_update_slow;
        break;
    case 3:
        PORTA &= ~pba_off_led1_reg;
        PORTA ^= pba_on_led1_reg;
        OCR1A += timer_blink_update_fast;
        break;
    case 4:
        PORTA &= ~pba_off_led1_reg;
        PORTA |= pba_on_led1_reg;
        break;
    }
    if (failure) {
        PORTA |= pba_on_led2_reg;
    }
    else {
        PORTA &= ~pba_on_led2_reg;
    }
}

void poll_pbas() {
    if (b_on.debounce()) {
        pba_on = true;
    }
    if (b_off.debounce()) {
        pba_off = true;
    }
    /*if (pba_on && pba_off) {
        debug = true;
    }*/
}

void start_sim() {
    /*for (Projector_wrapper* x : ser_con) {
        if (!(*x).proj_pwr_on()) {
            failure = true;
        }
    }*/
    if (proj_1.proj_pwr_on()) {
        failure = true;
        Serial.println("no fail");
    }
    Serial.print("sim_state: ");
    Serial.println(sim_state);
    sim_state = 2;
    Serial.print("sim_state: ");
    Serial.println(sim_state);
    bool cont = false;
    while (!cont) {
        cli();
        uint8_t sum = 0;
        for (Projector_wrapper*& x : ser_con) {
            if ((*x).ready_for_signal()) { sum += 1; }
        }
        if (sum == number_of_proj) { cont = true; }
        else { delay_c(1000); }
    }
    sim_state = 3;
    Serial.print("sim_state: ");
    Serial.println(sim_state);
    if (!failure) {
        failure = !start_main();
        if (!failure) {
            sim_state = 4;
            Serial.print("sim_state: ");
            Serial.println(sim_state);
        }
    }
}

bool start_main() {
    digitalWrite(main_pwr, HIGH);
    return(true);
}

void start_proj(uint8_t& x) {
    Serial.println("in start_proj");
    bool success = false;
    uint8_t tries = 0;
    //send start and wait for pass
    while (!success && tries < 3) {
        if ((*ser_con[x]).proj_pwr_on()) { success = true; }
        delay_c(50);
        tries++;
    }
    Serial.print("tries: ");
    Serial.println(tries);
    if (success) { Serial.println("success y"); }
    //check for power response(?)
    if (!success) {
        failure = true;
    }
}

void stop_sim() {
    sim_state = 1;
    PORTA &= ~pba_on_led1_reg;
    failure = !stop_main();
    for (uint8_t i = 0; i <= number_of_proj; i++) {
        if (!(*ser_con[i]).proj_pwr_off()) {
            failure = true;
        }
    }
    delay_c(cooldown_time);
    sim_state = 0;
}

bool stop_main() {
    digitalWrite(main_pwr, LOW);
    return(true);
}

bool reset_main() {
    digitalWrite(main_rst, LOW);
    return(true);
}

void status_sim() {
    uint8_t rfs_count = 0;
    bool rfs = false;
    for (Projector_wrapper*& x : ser_con) {
        if ((*x).ready_for_signal()) {
            rfs_count++;
        }
    }
    if (rfs_count == number_of_proj) {
        rfs = true;
    }
    if (sim_state == 4 && !digitalRead(main_feedback) && rfs) {
        sim_state = 3;
    }
    if (digitalRead(main_feedback) && !rfs) {
            failure = true;
    }
}

void setup()
{
    //setup blink timer1
    TCCR1A = 0;
    TCCR1B = 0;
    TCCR1B |= B00000100;
    OCR1A = timer_blink_update_slow;
    TIMSK1 |= B00000010;
    b_on.begin(pba_on_pin);
    b_off.begin(pba_off_pin);
    ser_con[0] = &proj_1;
    if (number_of_proj > 1) {
        ser_con[1] = &proj_2;
    }
    if (number_of_proj > 2) {
        ser_con[2] = &proj_3;
    }
    for (Projector_wrapper* x : ser_con) {
        (*x).begin_serial();
    }
    pinMode(pba_on_led1, OUTPUT);
    pinMode(pba_on_led2, OUTPUT);
    pinMode(pba_off_led1, OUTPUT);
    pinMode(pba_off_led2, OUTPUT);
    pinMode(main_pwr, OUTPUT);
    pinMode(main_rst, OUTPUT);
    pinMode(main_feedback, INPUT);
    Serial.begin(9600);
}

void loop()
{
    poll_pbas();
    if (pba_on && (sim_state == 0)) {
        start_sim();
    }
    else if (pba_off) { stop_sim(); }
    status_sim();
    //measure command latency
    //String bufdly = "";
    //bool suc = false;
    //delay(2000);
    //unsigned int enadly = millis();
    //proj_1.write_word("~00150 1\r");
    //while (!suc) {
    //    bufdly = proj_1.get_buffer();
    //    if (bufdly[0] == 'O') { suc = true; }
    //    else { delay(2); }
    //}
    //enadly = millis() - enadly;
    //Serial.println(enadly);
}
#ifndef Projector_wrapper_h
#define Projector_wrapper_h
#define buffer_size 18

#include <Arduino.h>


class Projector_wrapper{
  public:
    Projector_wrapper(uint8_t x) : portnr(x){};
    bool proj_pwr_on();
    bool proj_pwr_off();
    bool ready_for_signal();
    bool exists();
    String get_buffer();
    void begin_serial();
    void write_word(String txword);
  private:
    bool alive = false;
    void read_into_rxbuffer();
    void clear_rxbuffer();
    HardwareSerial* projector = nullptr;
    String rxbuffer = "";
    char txbuffer[buffer_size] = "";
    unsigned int lamp_hours = 0;
    uint8_t portnr = 0;
    bool rfs = false;

    static constexpr char cmd_pwr_on[] = "~0000 1\r";
    static constexpr char cmd_pwr_off[] = "~0000 0\r";
    static constexpr char cmd_pwr_stat[] = "~00124 1\r";
    static constexpr char cmd_info_stat[] = "~00150 1\r";

};

#endif



void inline delay_c(unsigned int time) {
    unsigned int cont = millis() + time;
    while (millis() <= cont) {}
}
#include "Projector_wrapper.h"

constexpr char Projector_wrapper::cmd_pwr_on[];
constexpr char Projector_wrapper::cmd_pwr_off[];
constexpr char Projector_wrapper::cmd_pwr_stat[];
constexpr char Projector_wrapper::cmd_info_stat[];


void Projector_wrapper::begin_serial(){
  switch (portnr){
    case 1:
      Serial1.begin(9600);
      projector = &Serial1;
      break;
    case 2:
      Serial2.begin(9600);
      projector = &Serial2;
      break;
    case 3:
      Serial3.begin(9600);
      projector = &Serial3;
      break;
  }
}

/*void Projector_wrapper::write_word(String txword){
  (*projector).write((char[])txword);
}*/

void Projector_wrapper::clear_rxbuffer(){
  rxbuffer = "";
}

void Projector_wrapper::read_into_rxbuffer(){
    clear_rxbuffer();
    while((*projector).available()){
        rxbuffer.concat((char)(*projector).read());
    }
}

void Projector_wrapper::write_word(String txword) {
    (*projector).print(txword);
}

bool Projector_wrapper::proj_pwr_on(){
    Serial.println("pwr_on 1");
    if (rfs) {
        Serial.write("rfs true");
        return true;
    }
    Serial.println("pwr_on 2");
    clear_rxbuffer();
    Serial.println("pwr_on 3");
    (*projector).write(cmd_pwr_on);
    Serial.println("pwr_on 4");
    delay_c(50);
    Serial.println("pwr_on 5");
    read_into_rxbuffer();
    Serial.println("pwr_on 6");
    if (!rxbuffer[0] == 'P'){
        Serial.println("pwr_on 7");
        return false;
    }
    Serial.println("pwr_on 8");
    return true;
}

bool Projector_wrapper::proj_pwr_off(){
    rfs = false;
    clear_rxbuffer();
    (*projector).write(cmd_pwr_off);
    delay_c(150);
    read_into_rxbuffer();
    if(rxbuffer[0]=='P'){
        return true;
    }
    return false;
}

bool Projector_wrapper::ready_for_signal(){
    clear_rxbuffer();
    (*projector).write(cmd_info_stat);
    delay_c(150);
    read_into_rxbuffer();
    if(rxbuffer[2]=='1'){
        rfs = true;
        return true;
    }
    return false;
}

bool Projector_wrapper::exists() {
    clear_rxbuffer();
    (*projector).write(cmd_info_stat);
    delay_c(150);
    read_into_rxbuffer();
    if (rxbuffer[0] == 'O') {
        alive = true;
        return true;
    }
    alive = false;
    return false;
}

String Projector_wrapper::get_buffer(){
    while ((*projector).available()) {
        rxbuffer.concat((char)(*projector).read());
    }
    return(rxbuffer);
}

why do you make your life complicated with ports, interrupts etc... ?

can't you just use a button class (such as Button in easyRun or OneButton or Toggle or EasyButton or Bounce2, ...)

1 Like

My buttons are in a button-class for exactly this reason. I neglected to post it to not bloat unnecessary, but here you go:

#ifndef button_h
#define button_h

#include "Arduino.h"

class Button
{
private:
    uint8_t btn;
    uint16_t state;
public:
    void begin(uint8_t button) {
        btn = button;
        state = 0;
        pinMode(btn, INPUT_PULLUP);
    }
    bool debounce() {
        state = (state << 1) | digitalRead(btn) | 0xfe00;
        return (state == 0xff00);
    }
};
#endif

strange and probably unreliably method to debounce buttons

You did not explain your reason. Or do you really mean you put the buttons into a button-class to make life complicated?

How should somebody carefully analyse a code if not all details can be seen??
Can you explain this to me??

Using serial print in a interrupt service routine shows that you are nearer to a beginner than to a professional code-developper.

Is this some kind of exercise trying to write code that uses interrupts in combination with 1. direct register manipulation ?
or
2. Do you just want to have some kind of functionality that is not highly time-critical?

If you want 2. = some kind of functionality where this functionality is not highly time-critical
start from scratch using standard commands like digitalWrite() and digitalRead()

best regards Stefan

Hey,
thanks for the advises. I actually used this debounce class I found after researching several debounce methods as it is working without using any classical blocking delay(). It works fine, did several test scripts with it.

The serial output in the ISR is only for debugging, as my external debugger (working with vmicro) seems not to catch proper breakpoints in an ISR and I had no time to investigate even another smallish issue, so I just ended up slapping serial writes everywhere as debug use. That will all be thrown out for the final version and at the beginning, it was all nice and well encapsulated in a clean "#ifdef debug - #endif".
Main reason for doing direct register manipulation was to avoid the performance overhead for digitalwrite und digitalread so that I have a consistent cycling frequency of the output, not varying by 50ms here and there. You see that in other parts of the code I do not mind using dw/dr.

But my main issue is still the exit of the function. I do not understand why it suddenly hangs. Originally I was also not even doing all the serial output there, just changing the sim_state global variable, but that never happened as the crash is before. Do I perform any invalid operation there somewhere, an unfinished pointer or so?

I don't know because I avoid using pointers. You have to use pointers very very carefully and pointers are 98% of all reasons making code act strange

You need to get out more. It is a well known technique, some ppl's goto. To function it must be called relatively slowly. Hanging it on a timer is the usual method to do, or a delay of 200 us (microseconds) can be used if the loop is so unburdened as to be too fast.


LOL! What else do you avoid that also causes 98% of all reasons making code act strange?

a7

4 Likes

The reason why you added prints to the interrupts is not important - but it can be the reason why your program freezes.
The fact is that the serial print procedures themselves use interrupts and accessing them from another interrupt can lead to an dead loop state.

Well your timing depends on how fast you call the debouncing function. On a fast or slow MCU or with a busy or less busy loop you’ll await for stabilization more or less…

This has been taken into account now so you won’t get a dead loop in the Serial class but indeed it’s a bad practice as print can block until bytes become available if the output buffer is full thus making the ISR lasts longer than needed (which can have detrimental consequences)

a simpler approach

// check multiple buttons and toggle LEDs

enum { Off = HIGH, On = LOW };

byte pinsLed [] = { 10, 11, 12 };
byte pinsBut [] = { A1, A2, A3 };
#define N_BUT   sizeof(pinsBut)

byte butState [N_BUT];

// -----------------------------------------------------------------------------
int
chkButtons ()
{
    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        byte but = digitalRead (pinsBut [n]);

        if (butState [n] != but)  {
            butState [n] = but;

            delay (10);     // debounce

            if (On == but)
                return n;
        }
    }
    return -1;
}

// -----------------------------------------------------------------------------
void
loop ()
{
    switch (chkButtons ())  {
    case 2:
        digitalWrite (pinsLed [2], ! digitalRead (pinsLed [2]));
        break;

    case 1:
        digitalWrite (pinsLed [1], ! digitalRead (pinsLed [1]));
        break;

    case 0:
        digitalWrite (pinsLed [0], ! digitalRead (pinsLed [0]));
        break;
    }
}

// -----------------------------------------------------------------------------
void
setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < sizeof(pinsBut); n++)  {
        pinMode (pinsBut [n], INPUT_PULLUP);
        butState [n] = digitalRead (pinsBut [n]);
    }

    for (unsigned n = 0; n < sizeof(pinsLed); n++)  {
        digitalWrite (pinsLed [n], Off);
        pinMode      (pinsLed [n], OUTPUT);
    }
}

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.