Signal pattern with ARDUSTIM

Hello folks. I'm using the ardustim project to generate the signal pattern of a specific phonic wheel. The code is the base code of the ardustim project. I only need to change the RPM of the wheel (the speed at which the pattern is generated) and change between two different wheel patterns. So i have 3 buttons and one selector: up, down and unit (if the units, tens, houndreds or thousands are changed) and the "selector" sets wheel 1 or wheel 2. Everything works fine but, SOMETIMES, when i change from one wheel to another, the pattern gets crazy and it starts generating a random signal. To recover the normal state the Arduino has to be restarted. Anyone knows what could be the issue?


Here is the first pattern, SOMETIMES when changing to the second wheel pattern the signal starts doing this random:

// ------------------ -----------------
// -------------------------------------
// Create button instances
#include <Bounce2.h>
// Create button instances
Bounce button1 = Bounce(); 
Bounce button2 = Bounce(); 
Bounce button3 = Bounce(); 
Bounce button4 = Bounce(); 

#define BUTTON_PIN1 2
#define BUTTON_PIN2 3
#define BUTTON_PIN3 4
#define BUTTON_PIN4 5
int n_button2 = 0;

typedef enum {
  UNITS,
  TENS,
  HUNDREDS,
  THOUSANDS
} State;
State currentState = UNITS;


// Create display instance and configuration
#include <Adafruit_SSD1306.h>
#include <Adafruit_GFX.h>
#define SCREEN_WIDTH          128   // OLED display width, in pixels
#define SCREEN_HEIGHT         64    // OLED display height, in pixels
#define OLED_RESET            -1    // Reset pin # (or -1 if sharing Arduino reset pin)
Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &Wire, OLED_RESET);

int     delta   = 1;
int     line    = 92; // starting line
uint8_t cursor;       // cursor position
char    buf[80];      // buffer data for the string name of the wheels
uint8_t px_tot = 105; // Alignement of RPM to the 105th pixel in X-axis
uint16_t w;




// -------------------------------------
// -------------------------------------

#include "defines.h"
#include "ardustim.h"
#include "enums.h"
#include "comms.h"
#include "storage.h"
#include "user_defaults.h"
#include "wheel_defs.h"
#include <avr/pgmspace.h>
#include <EEPROM.h>

/* Sensistive stuff used in ISR's */
// volatile uint8_t fraction = 0;





volatile bool reset_prescaler = false;
volatile bool normal = true;
volatile bool sweep_reset_prescaler = true; /* Force sweep to reset prescaler value */
volatile bool sweep_lock = false;
volatile uint8_t output_invert_mask = 0x00; /* Don't invert anything */
// volatile uint8_t sweep_direction = ASCENDING;
volatile byte total_sweep_stages = 0;
volatile uint8_t sweep_stage = 0;
volatile uint8_t prescaler_bits = 0;
volatile uint8_t last_prescaler_bits = 0;
volatile uint8_t mode = 0;
volatile uint16_t new_OCR1A = 1000; // sane default
volatile uint16_t edge_counter = 0;

/* Less sensitive globals */
uint8_t bitshift = 0;
//uint16_t sweep_low_rpm = 250;
//uint16_t sweep_high_rpm = 4000;
//uint16_t sweep_rate = 1;                               

sweep_step *SweepSteps;  /* Global pointer for the sweep steps */

wheels Wheels[MAX_WHEELS] = {
   /* Pointer to friendly name string, pointer to edge array, RPM Scaler, Number of edges in the array, whether the number of edges covers 360 or 720 degrees */
  { dizzy_four_cylinder_friendly_name, dizzy_four_cylinder, 0.25, 20, 90 },
  { dizzy_six_cylinder_friendly_name, dizzy_six_cylinder, 0.1666, 16, 360 },
};


/* Initialization */
void setup() {
  serialSetup();
  loadConfig();
  
  // Configure input of the button (2-> LEFT, 3, -> RIGHT, 4-> INCREASE, 5-> SWITCH)
  // Configure input of the button
  DDRD  = 0;                  // Reset values of port D data direction register
  DDRD  &= ~(1 << DDD2) & ~(1 << DDD3) & ~(1 << DDD4) & ~( 1 << DDD5);      // Set port 2, 3, 4, 5 as input
  // Pullup resistor
  PORTD = 0;                  // Reset values of port D data register
  PORTD |= (1 << PORTD2) | (1 << PORTD3) | (1 << PORTD4) | (1 << PORTD5);   // Enable pull-up resistor  

  button1.attach(BUTTON_PIN1);
  button2.attach(BUTTON_PIN2);
  button3.attach(BUTTON_PIN3);
  button4.attach(BUTTON_PIN4);
  button1.interval(50); // interval in ms
  button2.interval(50); // interval in ms
  button3.interval(50);
  button4.interval(50);


  // -------- display---------------------
  if(!display.begin(SSD1306_SWITCHCAPVCC, 0x3c)) {
  //if(!display.begin(SSD1306_SWITCHCAPVCC, 0x7A)) {
    Serial.println(F("SSD1306 allocation failed"));
    for(;;);
  }
  display.setTextColor(WHITE);
  display.setTextSize(2);

  display.clearDisplay();
  delay(2000);

  show_wheel_name();      // Show name of the wheel
  show_RPM();             // Show RPM
  

  // -------------------------------------
  // -------------------------------------

  config_timer1();

} // End setup



/* Pumps the pattern out of flash to the port 
 * The rate at which this runs is dependent on what OCR1A is set to
 * the sweeper in timer2 alters this on the fly to alow changing of RPM
 * in a very nice way
 */
ISR(TIMER1_COMPA_vect){
  /* This is VERY simple, just walk the array and wrap when we hit the limit */
  PORTB = output_invert_mask ^ pgm_read_byte(&Wheels[selected_wheel].edge_states_ptr[edge_counter]);   /* Write it to the port */
  /* Normal direction  overflow handling */
  if (normal){
    edge_counter++;
    if (edge_counter == Wheels[selected_wheel].wheel_max_edges){
      edge_counter = 0;
    }
  }
  
  /* The tables are in flash so we need pgm_read_byte() */

  /* Reset Prescaler only if flag is set */
  if (reset_prescaler){
    TCCR1B    &= ~((1 << CS10) | (1 << CS11) | (1 << CS12)); /* Clear CS10, CS11 and CS12 */
    TCCR1B    |= prescaler_bits;
    reset_prescaler = false;
  }
  /* Reset next compare value for RPM changes */
  OCR1A = new_OCR1A;  /* Apply new "RPM" from Timer2 ISR, i.e. speed up/down the virtual "wheel" */
}



void loop() {
  uint16_t tmp_rpm = 0;

  button1.update();
  button2.update();
  button3.update();
  button4.update();

  // Change wheel pattern
  if (button4.fell()){
    if (selected_wheel == 1){
      selected_wheel = 0;
    }else{
      selected_wheel = 1;
    }
    config_timer1();
    setRPM(wanted_rpm);

    show_wheel_name();      // Show name of the wheel
    show_RPM();             // Show RPM
  }

  // INCREASE BUTTON
  if (button2.fell()){
    wanted_rpm += delta;
    if (wanted_rpm >= 4000 || wanted_rpm<= 50){
      wanted_rpm = 4000;
    }
    setRPM(wanted_rpm);

    show_wheel_name();      // Show name of the wheel
    show_RPM();             // Show RPM
  }

  // DECREASE BUTTON
  if (button1.fell()){
    wanted_rpm -= delta;
    if (wanted_rpm < 50 || wanted_rpm >= 4000){
      wanted_rpm = 50;
    }
    setRPM(wanted_rpm);
    
    show_wheel_name();      // Show name of the wheel
    show_RPM();             // Show RPM
  }

  // Change UNITS
  if (button3.fell()){
    currentState = (currentState == THOUSANDS) ? UNITS : (State)((int)currentState + 1);
    switch(currentState){
      case UNITS:
        delta = 1;
        line  = 92;
        break;
      case TENS:
        delta = 10;
        line  = 80;
        break;
      case HUNDREDS:
        delta = 100;
        line  = 68;
        break;
      case THOUSANDS:
        delta = 1000;
        line  = 56;
        break;
    }
    
    
    show_wheel_name();      // Show name of the wheel
    show_RPM();             // Show RPM
  }
}



void config_timer1(){
  cli(); // stop interrupts

  // Configure pin 8 and 9 as outputs
  DDRB  = 0;
  DDRB  |= (1 << DDB0) && (1 << DDB1);
  PORTB = 0;

  /* Configuring TIMER1 (pattern generator) */
  // Set timer1 to generate pulses
  TCCR1A  = 0;
  TCCR1B  = 0;
  TCNT1   = 0;

  // Set compare register to sane default
  OCR1A   = 1000;  /* 8000 RPM (60-2) */

  // Turn on CTC mode
  TCCR1B  |= (1 << WGM12); // Normal mode (not PWM)
  // Set prescaler to 1
  TCCR1B  |= (1 << CS10); /* Prescaler of 1 */
  // Enable output compare interrupt for timer channel 1 (16 bit)
  TIMSK1  |= (1 << OCIE1A);
  
  // Turn on CTC mode
  TCCR1B |= (1 << WGM12); // Normal mode (not PWM)
  // Set prescaler to 1
  TCCR1B |= (1 << CS10); /* Prescaler of 1 */
  // Enable output compare interrupt for timer channel 1 (16 bit)
  TIMSK1 |= (1 << OCIE1A);

  sei(); // Enable interrupts
  
  reset_new_OCR1A(wanted_rpm);
}


void show_wheel_name(){
  display.clearDisplay();
  display.setCursor(0, 0);
  strcpy_P(buf, Wheels[selected_wheel].decoder_name);
  display.print(buf);
}


uint8_t get_txt_rpm_width(uint8_t wanted_rpm){
  int16_t x, y;
  uint16_t h;
  display.getTextBounds(String(wanted_rpm), 0, 0, &x, &y, &w, &h);
  return w;
}


void show_RPM(){
  // Print "RPM:"
  display.setCursor(9, 47);
  display.print("RPM:");  

  // Print rpm numerical value
  display.print(wanted_rpm);
  px_tot = get_txt_rpm_width(wanted_rpm);
  display.setCursor(px_tot - w, 47);
  display.drawLine(line, 63, line + 12, 63, SSD1306_WHITE);
  display.display();
}


void reset_new_OCR1A(uint32_t new_rpm){
  uint32_t tmp;
  uint8_t bitshift;
  uint8_t tmp_prescaler_bits;
  tmp = (uint32_t)(8000000.0/(Wheels[selected_wheel].rpm_scaler * (float)(new_rpm < 10 ? 10:new_rpm)));

  get_prescaler_bits(&tmp, &tmp_prescaler_bits, &bitshift);

  new_OCR1A       = (uint16_t)(tmp >> bitshift); 
  prescaler_bits  = tmp_prescaler_bits;
  reset_prescaler = true; 
}


uint8_t get_bitshift_from_prescaler(uint8_t *prescaler_bits){
  switch (*prescaler_bits){
    case PRESCALE_1024:
    return 10;
    case PRESCALE_256:
    return 8;
    case PRESCALE_64:
    return 6;
    case PRESCALE_8:
    return 3;
    case PRESCALE_1:
    return 0;
  }
  return 0;
}

//! Gets prescaler enum and bitshift based on OC value
void get_prescaler_bits(uint32_t *potential_oc_value, uint8_t *prescaler, uint8_t *bitshift){
  if (*potential_oc_value >= 16777216){
    *prescaler  = PRESCALE_1024;
    *bitshift   = 10;
  }
  else if (*potential_oc_value >= 4194304){
    *prescaler  = PRESCALE_256;
    *bitshift   = 8;
  }
  else if (*potential_oc_value >= 524288){
    *prescaler  = PRESCALE_64;
    *bitshift   = 6;
  }
  else if (*potential_oc_value >= 65536){
    *prescaler  = PRESCALE_8;
    *bitshift   = 3;
  }
  else{
    *prescaler  = PRESCALE_1;
    *bitshift   = 0;
  }
}

I moved your topic to an appropriate forum category @rabinobomba.

In the future, please take some time to pick the forum category that best suits the subject of your topic. There is an "About the _____ category" topic at the top of each category that explains its purpose.

This is an important part of responsible forum usage, as explained in the "How to get the best out of this forum" guide. The guide contains a lot of other useful information. Please read it.

Thanks in advance for your cooperation.

Since you didn't post all the include files, nobody can compile/test your code. The one item I did notice is that your have this

volatile uint16_t new_OCR1A = 1000; // sane default

but you cannot read/write a 16bit value atomically so changing that value has the possibility of messing things up. You have to turn off interrupts when accessing it outside the ISR

void reset_new_OCR1A(uint32_t new_rpm) {
  uint32_t tmp;
  uint8_t bitshift;
  uint8_t tmp_prescaler_bits;
  tmp = (uint32_t)(8000000.0 / (Wheels[selected_wheel].rpm_scaler * (float)(new_rpm < 10 ? 10 : new_rpm)));

  get_prescaler_bits(&tmp, &tmp_prescaler_bits, &bitshift);
  noInterrupts();  // cli();
  new_OCR1A       = (uint16_t)(tmp >> bitshift);
  interrupts();  // sei();
  prescaler_bits  = tmp_prescaler_bits;
  reset_prescaler = true;
}

You also don't have to do direct port manipulation for everything. This

  // Configure input of the button (2-> LEFT, 3, -> RIGHT, 4-> INCREASE, 5-> SWITCH)
  // Configure input of the button
  DDRD  = 0;                  // Reset values of port D data direction register
  DDRD  &= ~(1 << DDD2) & ~(1 << DDD3) & ~(1 << DDD4) & ~( 1 << DDD5);      // Set port 2, 3, 4, 5 as input
  // Pullup resistor
  PORTD = 0;                  // Reset values of port D data register
  PORTD |= (1 << PORTD2) | (1 << PORTD3) | (1 << PORTD4) | (1 << PORTD5);   // Enable pull-up resistor

can be written as:

  // Configure input of the button (2-> LEFT, 3, -> RIGHT, 4-> INCREASE, 5-> SWITCH)
  // Configure input of the button
  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);
  pinMode(5, INPUT_PULLUP);

which is much easier to understand. There are several examples of this throughout your code.

Hello @blh64 , thanks for your response.
I tried turning off the interrupts and enabling it later, as you said, but the performance is the same. When changing from one pattern to another, sometimes gets crazy and starts throwing random signals. The source of the code should come from another place, but I do not identify it.

Regarding the direct port manipulation, is made like that just to improve the speed, but it is true that the result is the same.

The discussed code is in the setup() and runs only once, so it is no much sense to improve its speed at the expense of readability.

Hello @b707 , true, but however, I do not think that this is the source of the problem I am trying to solve.

True. It is not the source of your issue, but having difficult to read/follow/understand code is not going to improve your chances of having someone figure out the issue nor help you revisit the code in the future

Hi @blh64 , yes, would be better to use the arduino preprogramed functions to make it easier to read, but since the ardustim project was made with direct hardware programming i just keep following the same "idea".

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