Simplifying Code - 8x8 LED Gear Shift Indicator

marco_c:
It simply reserves 2 bytes for the string (the character and the terminating '\0'). the memory would be laid out as P, \0, R, \0, N, \0, etc. It declares and array of NUM_GEARS of 2 byte character arrays.

Ah, I see! That makes sense, thank you - I think I'll use the first example just because that seems easier for me to understand:

const char GearChars[NUM_GEARS][2] = {"P", "R", "N", "D", "3", "2", "1"};

Deva_Rishi:
On the display yes, adding gears to the car is not going to get any easier though.

lol, that's true but for completeness I'll make sure it's included since adding or removing hall sensors is easy enough and accommodates for vehicles with different gear setups.

Deva_Rishi:
This part of the code is the bit that can be simplified of course. If you refer to the gears by number instead of by character and only refer to them by character [w]hen you need it.

I see what you mean, so if instead of:

char currentGear;

I would use:

int currentGear;

Then I can simply call the appropriate character using:

GearChars[currentGear]
//OR
GearChars[previousGear.last()]

and skip that loop since I already have their positions and can begin comparing?

Okay, so again taking the above on-board, here is what I now have:

#include <MD_Parola.h>
#include <MD_MAX72xx.h>
#include <SPI.h>
#include <CircularBuffer.h>

// constants won't change:
const int MAX_DEVICES = 1; // number of LED modules, should always be one
const int CLK_PIN = 13; // which pin on Arduino is wired to LED's CLK pin
const int DATA_PIN = 11; // which pin on Arduino is wired to LED's DATA pin
const int CS_PIN = 10; // which pin on Arduino is wired to LED's CS pin
const int NUM_HALL = 6; // set this to match the number of hall sensors
const int Hall[] = {3, 4, 5, 6, 7, 8}; // set to match wiring of sensors to their pins
const int NUM_GEARS = 7; // how many gears are used
// c-string for each gear, Parola lib requires for animation ([2] reserves 2 bytes for char and terminating '\O')
const char GearChars[NUM_GEARS][2] = {"P", "R", "N", "D", "3", "2", "1"}; // layout here from left to right should match gear order on vehicle from top to bottom
const char StartupText = "Startup Text"; //text scrolled upon boot
const int BRIGHTNESS = 4; // set brightness of LEDs (0-15), can replace with potentiometer-derived value
MD_Parola P = MD_Parola(CS_PIN, MAX_DEVICES);

// variables will change:
int hallState[NUM_GEARS];
int currentGear;
int hallTotal = 0;

CircularBuffer<int, 4> previousGear; // use a buffer to store last 4 gear positions
//4 used here as buffer will be used to detect 'sequence' such as 1-2-1-2, which can then be acted upon

// initialize the hall effect sensor pins as inputs:
void hallSetup() {
  for (int i = 0; i < NUM_HALL; i++) {
    pinMode(Hall[i], INPUT);
  }
}

// setup LED display
void displaySetup() {
  P.begin(); // initialise display
  P.setIntensity(BRIGHTNESS); //set display intensity/brightness (0-15)
  P.displayClear();
  P.displayText(StartupText, PA_CENTER, 100, 0, PA_SCROLL_LEFT, PA_SCROLL_LEFT); // display message on startup
  P.displayAnimate();
  if (P.displayAnimate()) { // If finished displaying message, reset and clear display
    P.displayReset();
    P.displayClear();
  }
}

// loop through all sensors once and record to array - includes 'fake' sensor as first value to represent 'P'
// then translates reading to relevant character by checking for non-zero value
int getGear() {
  for (int i = 1; i < NUM_HALL + 1; i++) {
    hallState[i] = digitalRead(Hall[i - 1]);
    if (hallState[i] == HIGH) {
      hallTotal = hallTotal + 1;
      previousGear.push(i); // stores current gear in buffer at the end, overwriting old data if necessary
      return i; //returns current loop iteration, which represents currentGear's location in GearChars array
    }
  }
  if (hallTotal == 0) { // if no HIGH readings, must be in 'P'
    previousGear.push(0); // stores current gear in buffer
    return 0; // returns 0 which represents 'P' location in GearChar array
  }
}

// displays current gear on LED, checks if animations should be used depending on previous (if any) gear value
void displayGear(int x) {
  if (previousGear.isEmpty() || x == previousGear.last()) { // if no previous value or same as previous, simply print
    P.displayText(GearChars[x], PA_CENTER, 0, 0, PA_PRINT, PA_NO_EFFECT);
    P.displayAnimate();
  }
  else { // if prev gear value exists AND is not the same as previous value
    // if the previous gear is situated to the left of current gear (in char array) then scroll up
    if (previousGear.last() < currentGear) {
      P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_UP, PA_NO_EFFECT);
      while (!P.displayAnimate())
        ;
      // if the previous gear is not situated left (i.e. is to the right of current gear in char array) then scroll down
    } else {
      P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_DOWN, PA_NO_EFFECT);
      while (!P.displayAnimate())
        ;
    }
  }
}

void setup() {
  hallSetup();
  displaySetup();
}

void loop() {
  currentGear = getGear();
  displayGear(currentGear);
}

All good?

It all compiles, so hopefully I can test it out soon to see if it behaves as expected.
Thank you all for your help!

As a side note too, in regards to the Parola library there are instructions on how to optimise the flash memory by removing unnecessary code here:

I figured that since I'm only using the basic PRINT, SCROLL_LEFT, SCROLL_UP, and SCROLL_DOWN functions and not mixing text and graphics then I would be okay to disable all the options suggested.
However, if I disable anything other than ENA_GRAPHICS, it fails to compile with the error(s):

Arduino: 1.8.13 (Windows 10), Board: "Arduino Uno"

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Fade.cpp:30:35: error: no 'void MD_PZone::effectFade(bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectFade(bool bIn)
                                   ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Dissolve.cpp:30:39: error: no 'void MD_PZone::effectDissolve(bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectDissolve(bool bIn)
                                       ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Random.cpp:32:37: error: no 'void MD_PZone::effectRandom(bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectRandom(bool bIn)
                                     ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Close.cpp:30:52: error: no 'void MD_PZone::effectClose(bool, bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectClose(bool bLightBar, bool bIn)
                                                    ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Mesh.cpp:30:35: error: no 'void MD_PZone::effectMesh(bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectMesh(bool bIn)
                                   ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Slice.cpp:30:36: error: no 'void MD_PZone::effectSlice(bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectSlice(bool bIn)
                                    ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Grow.cpp:30:45: error: no 'void MD_PZone::effectGrow(bool, bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectGrow(bool bUp, bool bIn)
                                             ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Blinds.cpp:32:37: error: no 'void MD_PZone::effectBlinds(bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectBlinds(bool bIn)
                                     ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Open.cpp:30:51: error: no 'void MD_PZone::effectOpen(bool, bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectOpen(bool bLightBar, bool bIn)
                                                   ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Diag.cpp:30:57: error: no 'void MD_PZone::effectDiag(bool, bool, bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectDiag(bool bUp, bool bLeft, bool bIn)
                                                         ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Scan.cpp:30:49: error: no 'void MD_PZone::effectHScan(bool, bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectHScan(bool bIn, bool bBlank)
                                                 ^

PATH\Arduino\libraries\MD_Parola\src\MD_Parola_Scan.cpp:115:49: error: no 'void MD_PZone::effectVScan(bool, bool)' member function declared in class 'MD_PZone'
 void MD_PZone::effectVScan(bool bIn, bool bBlank)
                                                 ^

exit status 1

Error compiling for board Arduino Uno.

Would anyone know why this would be?

I played a bit with the code from #15 and soon came to the same conclusion @Deva_Rishi describes in #19.

I also took out the CircularBuffer, its use here is clunky and whatever functionality it will ultimately support is easier to add to a program that just does the sensor/disply stuff. Just have a previous and a current number.

Also: do you not need to

while (!P.animate())
;

in the places where you don’t hold it in a while loop?

Like perhaps at the end of displaySetup, where you reset and clear the disaply if you’ve finished the animation, and you don’t if you haven’t. This is not what you want I think.

It won’t necessarily work directly in your sketch, but my irrepressible pattern recognizer rearranged your display function, viz:

/* display the current gear or */
/* animate the change to the new gear display if necessary */
void displayGear()
{
	textEffect_t scrollEffect;

	/* select effect */

	if (previousGear == currentGear)
		scrollEffect = PA_PRINT;
	else if (previousGear < currentGear)
		scrollEffect = PA_SCROLL_UP;
	else
		scrollEffect = PA_SCROLL_DOWN;

	P.displayText(gearChar[currentGear], PA_CENTER, 50, 1, scrollEffect, PA_NO_EFFECT);
	while (!P.displayAnimate())
			;

	previousGear = currentGear;		
	return;
}

A handy trick along the way for you is to initialize things into a known state so the rest of your code doesn’t need to have anything unique to the first time it is executed.

So after your splash screen animation finishes, just put the state of the entire mechanism you’ve created into PARK. Put it in current and previous and if you stick with the circular buffer, stuff PARK (0) in there, too. Soon enough you’ll be checking sensors and re-displaying, so no large deal to be in PARK briefly.

(I didn’t look close at the CircularBuffer, I assume can buffer whatever type you want.)

HTH better than working on my taxes, almost makes me want to have a reason to make a gear shift display…

a7

Thanks for taking a look! It’s very much appreciated, this whole thing has been a mixture of nightmare and ecstasy hahaha

alto777:
Also: do you not need to

while (!P.animate())
;

in the places where you don’t hold it in a while loop?

Like perhaps at the end of displaySetup, where you reset and clear the disaply if you’ve finished the animation, and you don’t if you haven’t. This is not what you want I think.

You’re right, I quickly realised this when the StartupText was being skipped!
I’ve updated that along with passing a HARDWARE_TYPE to MD_Parola as something has changed between the library versions to necessitate it.
It’s now displaying, and the StartupText is scrolling but the behaviour with gear switching is not behaving as expected.
Instead of starting with “P”, it shows “R” and the detection is sporadic and mostly incorrect.
I’ll try a version that incorporates the suggestions you’ve given - I like your idea of switching the print and scroll functions as it’s much simpler, but a couple of other variables change also and as far as I’m aware the displayAnimate is called differently.

I.e.:
To Print:

P.displayText(GearChars[x], PA_CENTER, 0, 0, PA_PRINT, PA_NO_EFFECT);
P.displayAnimate();

To Scroll:

P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_UP, PA_NO_EFFECT);
while (!P.displayAnimate())
    ;

Note that 0, 0 becomes 50, 1.

Is there anything you can see in my getGear or displayGear functions that would cause issues? I feel like I may have broken the logic somewhere along the way…

Latest:

#include <MD_Parola.h>
#include <MD_MAX72xx.h>
#include <SPI.h>
#include <CircularBuffer.h>

#define HARDWARE_TYPE MD_MAX72XX::GENERIC_HW
#define MAX_DEVICES 1 // number of LED modules, should always be one
#define CLK_PIN   13 // which pin on Arduino is wired to LED's CLK pin
#define DATA_PIN  11 // which pin on Arduino is wired to LED's DATA pin
#define CS_PIN    10 // which pin on Arduino is wired to LED's CS pin

// constants won't change:
const int NUM_HALL = 6; // set this to match the number of hall sensors
const int Hall[] = {3, 4, 5, 6, 7, 8}; // set to match wiring of sensors to their pins
const int NUM_GEARS = 7; // how many gears are used
// c-string for each gear, Parola lib requires for animation ([2] reserves 2 bytes for char and terminating '\O')
const char GearChars[NUM_GEARS][2] = {"P", "R", "N", "D", "3", "2", "1"}; // layout here from left to right should match gear order on vehicle from top to bottom
const char StartupText[] = "Startup Text"; //text scrolled upon boot
const int BRIGHTNESS = 4; // set brightness of LEDs (0-15), can replace with potentiometer-derived value
MD_Parola P = MD_Parola(HARDWARE_TYPE, CS_PIN, MAX_DEVICES);

// variables will change:
int hallState[NUM_GEARS];
int prevGear = 0;
int currentGear = 0;
int hallTotal = 0;

CircularBuffer<int, 4> previousGear; // use a buffer to store last 4 gear positions
//4 used here as buffer will be used to detect 'sequence' such as 1-2-1-2, which can then be acted upon

// initialize the hall effect sensor pins as inputs:
void hallSetup() {
  for (int i = 0; i < NUM_HALL; i++) {
    pinMode(Hall[i], INPUT);
  }
}

// setup LED display
void displaySetup() {
  P.begin(); // initialise display
  P.setIntensity(BRIGHTNESS); //set display intensity/brightness (0-15)
  P.displayClear();
  P.displayText(StartupText, PA_CENTER, 100, 0, PA_SCROLL_LEFT, PA_SCROLL_LEFT); // display message on startup
  while (!P.displayAnimate())
        ;
  P.displayReset();
  P.displayClear();
//  previousGear.push(0);
}

// loop through all sensors once and record to array - includes 'fake' sensor as first value to represent 'P'
// then translates reading to relevant character by checking for non-zero value
int getGear() {
  for (int i = 1; i < NUM_HALL + 1; i++) {
    hallState[i] = digitalRead(Hall[i - 1]);
    if (hallState[i] == HIGH) {
      hallTotal = hallTotal + 1;
      previousGear.push(i); // stores current gear in buffer at the end, overwriting old data if necessary
      return i; //returns current loop iteration, which represents currentGear's location in GearChars array
    }
  }
  if (hallTotal == 0) { // if no HIGH readings, must be in 'P'
    previousGear.push(0); // stores current gear in buffer
    return 0; // returns 0 which represents 'P' location in GearChar array
  }
}

// displays current gear on LED, checks if animations should be used depending on previous (if any) gear value
void displayGear(int x) {
  if (previousGear.isEmpty() || x == previousGear.last()) { // if no previous value or same as previous, simply print
    P.displayText(GearChars[x], PA_CENTER, 0, 0, PA_PRINT, PA_NO_EFFECT);
    P.displayAnimate();
  }
  else if (previousGear.last() < currentGear) { // if prev gear value exists AND is not the same as previous value
    // if the previous gear is situated to the left of current gear (in char array) then scroll up
    
      P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_UP, PA_NO_EFFECT);
      while (!P.displayAnimate())
        ;
      // if the previous gear is not situated left (i.e. is to the right of current gear in char array) then scroll down
    } else {
      P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_DOWN, PA_NO_EFFECT);
      while (!P.displayAnimate())
        ;
    }
  }
}

void setup() {
  hallSetup();
  displaySetup();
}

void loop() {
  currentGear = getGear();
  displayGear(currentGear);
}

Haha, my pattern recognizer failed to see those other differences. I don’t know if I would go to the trouble, but these can be accommodated by setting some local variables in addition to the textEffect_t variable in each section.

If you are using the word sporadic, perhaps it would be a good time to write a tiny program that does nothing but continuously report on the hall sensors. I haven’t used them, maybe it’s hardware sporadicity (!)

not software.

a7

alto777:
Haha, my pattern recognizer failed to see those other differences. I don’t know if I would go to the trouble, but these can be accommodated by setting some local variables in addition to the textEffect_t variable in each section.

If you are using the word sporadic, perhaps it would be a good time to write a tiny program that does nothing but continuously report on the hall sensors. I haven’t used them, maybe it’s hardware sporadicity (!)

not software.

a7

That's a great point!
I'll write up something simple to just output to serial tomorrow and see if there are any inconsistencies, and start from there.

In #24 version's getGear() this

if (hallTotal == 0) { // if no HIGH readings, must be in 'P'

is not necessary, no need for hallTotal or to count. hallTotal will always be zero at this point, as you took a return earlier if you had a sensor down.

Except…

Looking again to completely eliminate the variable (why global?) hallTotal, I note that you never reset it to zero before using it to count… which means if no sensor makes getGear take the early return, hallTotal won't be zero, so the if statement at the end will neither push to previousGear nor "return 0".

Asking: what does a function returning int do when it just falls off the end without executing any return (expr); ?

This could be a sporadegenic logic error. Time to turn up your compiler warnings and endeavour to eliminate them, even when you get the feeling you've been warned from an abundance of misunderstanding. Not in this case, getGear raises a flag that would have tipped you to this:

"warning: control reaches end of non-void function [-Wreturn-type]"

From your latest (after I removed a stray '{' or was it a '}'. :wink:

edit: Sry, I could have taken 12 seconds and said it's set in the Preferences, middle of the first screen, crank it up to all, pay attention to the red ink in the output window under your sketch after compilation.

a7

Somehow i don't see why you didn't go with this ?

int getGear() {
  int gear = 6;
  while ((gear >0) && (digitalRead(hall[gear - 1]))) { 
    gear--;
  }
  previousGear.push(gear);
  return gear;

Okay so I got some sleep and came back at it fresh, and I realised how many little mistakes I’d made when re-factoring.
Unexpected detection behaviour was due to checking for a HIGH reading instead of a LOW reading (stupid mistake!).

Deva_Rishi:
Somehow i don’t see why you didn’t go with this ?

int getGear() {

int gear = 6;
  while ((gear >0) && (digitalRead(hall[gear - 1]))) {
    gear–;
  }
  previousGear.push(gear);
  return gear;

Probably because it looked intimidating! You’re right though, and that also makes the hallState array superfluous.
I do like it, but in my inexperience I thought previously that it would give the wrong gear and it would need the GearChar array to be reversed as it looked to be looping ‘backwards’ - but I should’ve known you’d know better than I!

I hope this is the last iteration needed!
I’ve kept the buffer in because that’s my next step once the basics are ironed out, and it already saves me from using another prevGear variable - and I’m fairly confident in the implementation even if it is clunky for now.

Am I correct in thinking that the getGear is looking for a LOW (sensor output: 0) reading - as the sensor must be in HIGH (output: 1) if nothing is detected, which would make the statement:

(digitalRead(Hall[gear - 1])

TRUE, which then allows progression to the

gear--;

part of the code, and continuing the loop?

//necessary libraries, ensure they are installed in Arduino IDE before uploading
#include <MD_Parola.h>
#include <MD_MAX72xx.h>
#include <SPI.h>
#include <CircularBuffer.h>

#define HARDWARE_TYPE MD_MAX72XX::GENERIC_HW // change if using PAROLA_HW or other LED hardware, incorrect setting can cause orientation issues
#define MAX_DEVICES 1 // number of LED modules, should always be one in this setup - unsure if more would work with no issues
#define CLK_PIN   13 // LED's CLK pin
#define DATA_PIN  11 // LED's DATA pin
#define CS_PIN    10 // LED's CS pin

// constants won't change:
const int NUM_GEARS = 7; // how many gears are used - must match the number of gear characters in GearChars array
const int NUM_LOOPS = NUM_GEARS - 1;
const int Hall[] = {3, 4, 5, 6, 7, 8}; // sensor pins in gearChars array- 'Park' position is assumed to not have sensor
// c-string for each gear, Parola lib requires for animation ([2] reserves 2 bytes for char and terminating '\O')
const char GearChars[NUM_GEARS][2] = {"P", "R", "N", "D", "3", "2", "1"}; // layout here from left to right should match gear order on vehicle from top to bottom
const char StartupText[] = "Startup Text"; //text scrolled upon boot
const int BRIGHTNESS = 4; // set brightness of LEDs (0-15), can replace with potentiometer-derived value
MD_Parola P = MD_Parola(HARDWARE_TYPE, CS_PIN, MAX_DEVICES);

// variables will change:
int currentGear;

CircularBuffer<int, 4> previousGear; // use a buffer to store last 4 gear positions
//4 used here as buffer will be used to detect 'sequence' such as 1-2-1-2, which can then be acted upon

// initialize the hall effect sensor pins as inputs:
void hallSetup() {
  for (int i = 0; i < NUM_LOOPS; i++) {
    pinMode(Hall[i], INPUT);
  }
}

// setup LED display
void displaySetup() {
  P.begin(); // initialise display
  P.setIntensity(BRIGHTNESS); //set display intensity/brightness (0-15)
  P.displayClear();
  P.displayScroll(StartupText, PA_LEFT, PA_SCROLL_LEFT, 100); // display message on startup
  while (!P.displayAnimate())
    ;
  P.displayReset();
  P.displayClear();
}

// loop through sensors until LOW reading detected
int getGear() {
  int gear = NUM_LOOPS;
  while ((gear > 0) && (digitalRead(Hall[gear - 1]))) {
    gear--;
  }
  return gear;
}

// displays current gear on LED, checks if animations should be used depending on previous (if any) gear value
void displayGear(int x) {
  if (x == previousGear.last()) { // if current gear is same as previous, simply print
    P.displayText(GearChars[x], PA_CENTER, 0, 0, PA_PRINT, PA_NO_EFFECT); // set display settings
    P.displayAnimate(); // display appropriate character
  }
  // if the previous gear is situated to the left of current gear (in char array) then scroll up
  else if ((previousGear.last() < x)) { // if current gear is not the same as previous value
    P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_UP, PA_NO_EFFECT); // set scroll effect
    while (!P.displayAnimate()) // play animation
      ;
    // if the previous gear is not situated left (i.e. is to the right of current gear in char array) then scroll down
  } else {
    P.displayText(GearChars[x], PA_CENTER, 50, 1, PA_SCROLL_DOWN, PA_NO_EFFECT);
    while (!P.displayAnimate())
      ;
  }
  previousGear.push(x); // push current gear to buffer
}

void setup() {
  hallSetup(); // initialise sensors
  displaySetup(); // initialise display
  currentGear = 0; // set current gear to 'Park' position until first sensor read to establish known state
  previousGear.push(0); // push 'Park' position to buffer also
}

void loop() {
  currentGear = getGear(); // read hall effect sensors and calculate current gear position
  displayGear(currentGear); // display the current gear, with animation if different from previous gear
}

EDIT:
Thanks everyone for your help!
With integrated debugging, posted for completeness:

/* necessary libraries, ensure they are
  installed in Arduino IDE before uploading */
#include <MD_Parola.h>
#include <MD_MAX72xx.h>
#include <SPI.h>
#include <CircularBuffer.h>

#define HARDWARE_TYPE MD_MAX72XX::GENERIC_HW                                     // change if using PAROLA_HW or other LED hardware, incorrect setting can cause orientation issues

/* settings: */
const uint8_t MAX_DEVICES             = 1;                                       // number of LED modules, should always be one in this setup - unsure if more would work with no issues
const uint8_t CLK_PIN                 = 13;                                      // LED's CLK pin
const uint8_t DATA_PIN                = 11;                                      // LED's DATA pin
const uint8_t CS_PIN                  = 10;                                      // LED's CS pin
const int8_t  NUM_GEARS               = 7;                                       // how many gears are used - must match the number of gear characters in GearChars array
const char    GearChars[NUM_GEARS][2] = {"P", "R", "N", "D", "3", "2", "1"};     // layout here from left to right should match gear order on vehicle from top to bottom/start to finish
const uint8_t Hall[]                  = {      3,   4,   5,   6,   7,   8 };     // sensor pins in the same order as gearChars array - 'Park' position is assumed to not have a sensor and so first pin is "R"
const char    StartupText[]           = {"Startup Text"};     // text scrolled upon boot
const uint8_t SCROLL_SPEED            = 75;                                      // speed that StartupText is scrolled, number is milliseconds between frame updates
const uint8_t BRIGHTNESS              = 4;                                       // set brightness of LEDs (0-15), can replace with potentiometer-derived value
const uint8_t DEBUG_MODE              = 0;                                       // set to 1 to enable debugging via Serial (baud)
const uint8_t BUFFER_SIZE             = 4;                                       // set number of stored previous gear states - 4 used here used to detect 'sequence' such as 1-2-1-2, which can then be acted upon
const uint8_t DEBUG_READS             = BUFFER_SIZE;                             // number of readings in debug mode, recommended to match buffer size or larger
const int     DEBUG_DELAY             = 3000;                                    // delay between debug readings in milliseconds (3 seconds by default)
const int     BAUD_SPEED              = 9600;                                    // set baud rate here for Serial communication
const int8_t  NUM_LOOPS               = NUM_GEARS - 1;                           // used for loop counting etc when starting with 0

MD_Parola P = MD_Parola(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);  // creates display instance using given settings

/* variables will change: */
int8_t currentGear;
CircularBuffer<int8_t, BUFFER_SIZE> previousGears;                               // a buffer to store previous gear positions

void setup() {
  hallSetup();                                                                   // initialise sensors
  displaySetup();                                                                // initialise display
  currentGear = 0;                                                               // set current gear to 'Parked' position until first sensor read to establish known state
  previousGears.push(currentGear);                                               // push 'Park' {"P"} position to buffer also, which is translated to *char via GearChars[0]
  if (DEBUG_MODE == 1) {                                                         // check if
    Serial.begin(BAUD_SPEED);
    debugFunction();
  }
}

void loop() {
  currentGear = getGear();                                                       // read hall effect sensors and calculate current gear position
  displayGear(currentGear);                                                      // display the current gear, with appropriate animation if different from previous gear
}

/* writes 4 (or DEBUG_READS) lots of readings from
  all sensors to Serial, with a delay to allow changing
  gear, and fills & prints buffer for debugging purposes */
void debugFunction() {
  String buf = "";
  for (int8_t i = 0; i < DEBUG_READS; i++) {
    delay(DEBUG_DELAY);                                                          // wait to allow gear changing/hall sensor/magnet position changes
    for (int8_t x = 0; x < NUM_LOOPS; x++) {                                     // loop through all sensors and print values to Serial
      Serial.println(
        String(x + 1) +
        "| Digital: " +
        String(digitalRead(Hall[x])) +
        " Analogue: " +
        String(analogRead(Hall[x]))
      );
    }
    previousGears.push(GearChars[random(NUM_LOOPS)]);                            // push pseudorandom GearChar values to buffer
    buf = buf + previousGears.last();                                            // add current gear to a string for printing to Serial
  }
  Serial.println("Buffer contents: " + buf);                                     // ...print buffer contents, to Serial...
  while (true);                                                                  // puts arduino into an infinite loop, reboot to start again
}

/* initialize the hall effect sensor pins as inputs: */
void hallSetup() {
  for (int8_t i = 0; i < NUM_LOOPS; i++) {
    pinMode(Hall[i], INPUT);
  }
}

/* setup LED display */
void displaySetup() {
  P.begin();                                                                     // initialise display
  P.setIntensity(BRIGHTNESS);                                                    // set display intensity/brightness
  P.displayClear();
  P.displayScroll(StartupText, PA_LEFT, PA_SCROLL_LEFT, SCROLL_SPEED);           // display message on startup
  while (!P.displayAnimate())                                                    // play animation once until complete
    ;
  P.displayReset();
  P.displayClear();
}

/* loop through sensors until LOW reading detected */
int8_t getGear() {
  int8_t gear = NUM_LOOPS;
  while ((gear) && (digitalRead(Hall[gear - 1]))) {
    gear--;
  }
  return gear;
}

/* displays current gear on LED, checks if animations
  should be used depending on previous gear value */
void displayGear(int8_t gearValue) {
  char curGearChar = GearChars[curGearChar];                                     // convert gearValue to c-string character for display purposes
  if (gearValue == previousGears.last()) {                                       // if current gear is same as previous, simply print
    P.displayText(curGearChar, PA_CENTER, 0, 0, PA_PRINT, PA_NO_EFFECT);         // set display settings
    P.displayAnimate();                                                          // display appropriate character
  }
  else if ((previousGears.last() < gearValue)) {                                 // if the previous gear is situated to the left of current gear (in char array) then scroll down
    P.displayText(
      curGearChar, PA_CENTER, SCROLL_SPEED, 1, PA_SCROLL_DOWN, PA_NO_EFFECT      // set scrolling text settings
      );  
    while (!P.displayAnimate())                                                  // play once animation until complete
      ;
  } else {                                                                       // if the previous gear is not situated left (i.e. is to the right of current gear in char array) then scroll up
    P.displayText(
      curGearChar, PA_CENTER, SCROLL_SPEED, 1, PA_SCROLL_UP, PA_NO_EFFECT
      );
    while (!P.displayAnimate())
      ;
  }
  previousGears.push(gearValue);                                                 // push current gear to buffer
}

You should test compile before you upload! And then carefully cut and paste…

this from 30 minutes past the hour

while true {

keeps what you posted from compikling. Time is too short to fix that and see if there is a next thing. Some of us are pretty old around here, so save us the time, haha.

a7

But since I am next turning to working on my taxes, I "fixed" it and I note:

Your debug function never breaks out of the while loop.

You could place the decision to spit out debugging at the very end of setup(); and leave your (previously) beautiful, concise and exemplary loop() function unsullied.

HTH

a7

alto777:
But since I am next turning to working on my taxes, I "fixed" it and I note:

Your debug function never breaks out of the while loop.

You could place the decision to spit out debugging at the very end of setup(); and leave your (previously) beautiful, concise and exemplary loop() function unsullied.

HTH

a7

Apologies, despite going back and doing a few edits the forum didn't show your reply until just now for me.
The infinite loop is intentional, to halt all action for analysis.

You're right about the loop(), thanks for that! I've shifted it as appropriate.

Am I correct in thinking that the getGear is looking for a LOW (sensor output: 0) reading - as the sensor must be in HIGH (output: 1) if nothing is detected, which would make the statement:
Code: [Select]
(digitalRead(Hall[gear - 1])

TRUE, which then allows progression to the
Code: [Select]
gear--;

part of the code, and continuing the loop?

Yes and yes ! until gear reaches '0' and the while loop is terminated. gear--; is 1 of 2 statements in the while loop (the other being digitalRead(hall[gear - 1]) which is executed as part of the condition. )
The note i made previously about the sequence of the 'conditions; is significantwhile ((digitalRead(hall[gear - 1])) && (gear >0))would not work because when gear reaches '0' the first statement reads element -1 of the array, which does not really exist. The Arduino should then crash, but it may just find something in that memory location and process that.
about the (gear >0) can also be written as (gear)if part of a condition (gear) == false if gear == 0, in all other cases (gear) == true;
the same goes for (digitalRead(Hall[gear - 1])which can also be seen as (digitalRead(Hall[gear - 1] == HIGH)
Since the hardware setup does not allow for more than 1 hall sensor being active, it is fine to just exit the loop if one is found to be active (causing the pin to go LOW) If for some strange reason there are 2 hall sensors active it will return the highest value.

Okay, so updating with animation check/display now breaks it somewhere :confused:

#include <MD_Parola.h>
#include <MD_MAX72xx.h>
#include <SPI.h>
#include <CircularBuffer.h>

#define HARDWARE_TYPE MD_MAX72XX::GENERIC_HW                                     

/* settings: */
const uint8_t MAX_DEVICES             = 1;                                       
const uint8_t CLK_PIN                 = 13;                                      
const uint8_t DATA_PIN                = 11;                                      
const uint8_t CS_PIN                  = 10;                                      
const int8_t  NUM_GEARS               = 7;                                       
const char    GearChars[NUM_GEARS][2] = {"P", "R", "N", "D", "3", "2", "1"};     
const uint8_t Hall[]                  = {      3,   4,   5,   6,   7,   8 };     
const char    StartupText[]           = {"Startup Text"};
const uint8_t SCROLL_SPEED            = 75;
const uint8_t BRIGHTNESS              = 4;                                       
const char    ANIM_SEQUENCE           = {"DNDN"};                                
const uint8_t DEBUG_MODE              = 0;                                       
const uint8_t BUFFER_SIZE             = 4;                                       
const uint8_t DEBUG_READS             = BUFFER_SIZE;                             
const int     DEBUG_DELAY             = 3000;                                    
const int     BAUD_SPEED              = 9600;                                    
const int8_t  NUM_LOOPS               = NUM_GEARS - 1;                           

MD_Parola P = MD_Parola(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);  

const uint8_t F_PMAN2 = 6;
const uint8_t W_PMAN2 = 18;
static const uint8_t PROGMEM pacman[F_PMAN2 * W_PMAN2] =                         // ghost pursued by pacman
{  "removed for brevity"
};

int8_t currentGear;
CircularBuffer<int8_t, BUFFER_SIZE> previousGears;                               

void setup() {
  hallSetup();                                                                   
  displaySetup();                                                                
  currentGear = 0;                                                               
  previousGears.push(currentGear);                                               
  if (DEBUG_MODE == 1) {                                                         
    Serial.begin(BAUD_SPEED);
    debugFunction();
  }
}

void loop() {
  currentGear = getGear();                                                       
  displayGear(currentGear);                                                      
  checkHistory();                                                                
}

void debugFunction() {
  String buf = "";
  for (int8_t i = 0; i < DEBUG_READS; i++) {
    delay(DEBUG_DELAY);                                                          
    for (int8_t x = 0; x < NUM_LOOPS; x++) {                                     
      Serial.println(
        String(x + 1) +
        "| Digital: " +
        String(digitalRead(Hall[x])) +
        " Analogue: " +
        String(analogRead(Hall[x]))
      );
    }
    previousGears.push(GearChars[random(NUM_LOOPS)]);                            
    buf = buf + previousGears.last();                                            
  }
  Serial.println("Buffer contents: " + buf);                                     
  while (true);                                                                  
}

void hallSetup() {
  for (int8_t i = 0; i < NUM_LOOPS; i++) {
    pinMode(Hall[i], INPUT);
  }
}

void displaySetup() {
  P.begin();                                                                     
  P.setIntensity(BRIGHTNESS);                                                    
  P.displayClear();
  P.displayScroll(StartupText, PA_LEFT, PA_SCROLL_LEFT, SCROLL_SPEED);           
  while (!P.displayAnimate())                                                    
    ;
  P.displayReset();
  P.displayClear();
}

int8_t getGear() {
  int8_t gear = NUM_LOOPS;
  while ((gear) && (digitalRead(Hall[gear - 1]))) {
    gear--;
  }
  return gear;
}

void displayGear(int8_t gearValue) {
  char curGearChar = GearChars[curGearChar];                                     
  if (gearValue == previousGears.last()) {                                       
    P.displayText(curGearChar, PA_CENTER, 0, 0, PA_PRINT, PA_NO_EFFECT);         
    P.displayAnimate();                                                          
  }
  else if ((previousGears.last() < gearValue)) {                                 
    P.displayText(
      curGearChar, PA_CENTER, SCROLL_SPEED, 1, PA_SCROLL_DOWN, PA_NO_EFFECT      
    );
    while (!P.displayAnimate())                                                  
      ;
  } else {                                                                       
    P.displayText(
      curGearChar, PA_CENTER, SCROLL_SPEED, 1, PA_SCROLL_UP, PA_NO_EFFECT
    );
    while (!P.displayAnimate())
      ;
  }
  previousGears.push(gearValue);                                                 
}

/* WIP */

/* checks for sequences of gear changes and calls other functions as required */
void checkHistory() {
  if (previousGears.isFull()) {
    char gearHistory = "";
    for (int8_t i = 0; i < BUFFER_SIZE; i++) {                                   // convert array to char string for use in switch
      gearHistory = (gearHistory + GearChars[previousGears[i]]);
    }
    if (gearHistory == ANIM_SEQUENCE) {                                          // if gear history equals ANIM_SEQUENCE then play animation by calling displayAnimation()
      displayAnimation();
    }
  }
}

/* scrolls pacman animation left then right */
void displayAnimation() {
  P.displayText(
    pacman, PA_CENTER, SCROLL_SPEED, 1, PA_SCROLL_LEFT, PA_NO_EFFECT             // set scroll settings (left)
  );
  while (!P.displayAnimate())                                                    // play once animation until complete
    ;
  P.displayText(
    pacman, PA_CENTER, SCROLL_SPEED, 1, PA_SCROLL_RIGHT, PA_NO_EFFECT            // set scroll settings (right)
  );
  while (!P.displayAnimate())                                                    // play once animation until complete
    ;
}

char curGearChar = GearChars[curGearChar];  This looks wrong for 2 reasons. First the variable passed to should be a null-terminated char array. (or at least that is what was decided a few pages ago) And second i am pretty sure that the index of GearChars should be 'gearvalue' (not the just created 'curGearChar' which is still '\0' )

char curGearChar[2];
curGearChar[0] = GearChars[gearvalue];

Deva_Rishi:

char curGearChar = GearChars[curGearChar];

This looks wrong for 2 reasons. First the variable passed to should be a null-terminated char array. (or at least that is what was decided a few pages ago)

I was aware of this, but thought that it was pulling the already null-terminated char from the array - you're saying that is incorrect and it's dropping the terminator because I didn't specify [2]?

And second i am pretty sure that the index of GearChars should be 'gearvalue' (not the just created 'curGearChar' which is still '\0' )

char curGearChar[2];

curGearChar[0] = GearChars[gearvalue];

That's the mistake I couldn't spot! Such a simple mistake, but one not shown by the compiler!
Thank you so much.

I was aware of this, but thought that it was pulling the already null-terminated char from the array - you're saying that is incorrect and it's dropping the terminator because I didn't specify [2]?

Well if Parola takes a c-string (char * or null terminated char array) then you should provide it with one. I don't know what it takes as an argument, but to be safe that would be better.

@rwj: To be clear, Parola needs to be give a nul ('\0') terminated array of characters. So you can either pass it

char *curGearChar = GearChars[curGearChar];

which is a pointer to the string in your lookup table, or

char curGearChar[2] = { 'x', '\0' }; // initialize temporary storage

curGearChar[0] = GearChars[curGearChar]; // overwrite first character in the string

where you are copying the character into a new string. Note you have to initialize the temporary array to include the nul character or it will be random stuff that was in memory.

Personally I would use the pointer to the lookup table because that why you use lookup tables, but your hybrid is not correct.