Unreliable ranking values using switch case

Using Arduino Project Guidance
#1

Hi, I am having some trouble with my code and wondered if anyone has an idea on how I can improve it..

The reading I am getting is somewhat not satisfying.

I am using 'max' and 'min' and 'switch case' to sort them in order then display to an LCD...

The problem I am getting is to do with the ranking refresh (it's gonna be tricky for me to explain):
Basically, the position1 or P1 etc, is not automatically updating to P2 as soon as another variable becomes greater than his.... Make sense? it takes a few numbers above to actually make the transition and it seem that greater the variable, the more gap you need between the variables to make a transition...

Long story short: this is part of the code that I am not happy with:

  int maximum = max(max(intOne, intTwo), max(intThree, intFour));
  int minimum = min(min(intOne, intTwo), min(intThree, intFour));

  int red = map(intOne, maximum, minimum, 0, 3);
  switch (red){
    case 0:
      redPos = 1;
      break;
    case 1:
      redPos = 2;
      break;
    case 2:
      redPos = 3;
      break;
    case 3:
      redPos = 4;
      break;
  }

  int green = map(intTwo, maximum, minimum, 0, 3);
  switch (green){
    case 0:
      greenPos = 1;
      break;
    case 1:
      greenPos = 2;
      break;
    case 2:
      greenPos = 3;
      break;
    case 3:
      greenPos = 4;
      break;
  }
  
  int yellow = map(intThree, maximum, minimum, 0, 3);
  switch (yellow){
    case 0:
      yellowPos = 1;
      break;
    case 1:
      yellowPos = 2;
      break;
    case 2:
      yellowPos = 3;
      break;
    case 3:
      yellowPos = 4;
      break;
  }
  
  int blue = map(intFour, maximum, minimum, 0, 3);
  switch (blue){
    case 0:
      bluePos = 1;
      break;
    case 1:
      bluePos = 2;
      break;
    case 2:
      bluePos = 3;
      break;
    case 3:
      bluePos = 4;
      break;
  }

  lcd.setCursor(0,0);
  lcd.print("P");
  lcd.setCursor(1,0);
  lcd.print(intOne);
  lcd.setCursor(0,1);
  lcd.print("P");
  lcd.setCursor(1,1);
  lcd.print(intTwo);
  lcd.setCursor(0,2);
  lcd.print("P");
  lcd.setCursor(1,2);
  lcd.print(intThree);
  lcd.setCursor(0,3);
  lcd.print("P");
  lcd.setCursor(1,3);
  lcd.print(intFour);

Thanks in advance for any help..

#2

What's the point of the switch case?
A simple ++ would seem to me to suffice.

#3

Thanks for your reply, I think I didn't express myself properly:

I am not trying to increment the integer ( as in intOne, intTwo, etc...)
I want to rank them in order, as in: the largest value on P1 and so on...
And finally update that value as soon as its ranking changes.

Which it seem to do the job, just not well enough..
Later on I also need to tiebreak matching values based on time.

#4

Your fault description doesn't make me understand. If You post the entire code I might get some more ....... Else You improve that snippet of code Yourself.

#5

Neither would I be :wink: Terribly complicated, something like below makes more sense to me

redpos = red+1;
#6

Thanks, it definitely made the code simpler.. but sadly, the reading result is the same..

I will keep on trying :slightly_smiling_face:

#7

I don't even understand your original problem. I don't see a ranking anywhere in your code. Maybe you should give it another try in explaining. And post your complete code.

#8

I'm really unclear what it is you're trying to do, but how about concentrating on just one channel, and printing intermediate results, particularly around the map function?

#9

There are four buttons that increment each a value (intOne to intFour), in fact: I've renamed those variables intOne, etc, as I thought it would have helped to understand the concept..

#include <LiquidCrystal_I2C.h>
#include <Wire.h>
LiquidCrystal_I2C lcd(0x3F,20,4);

int redLap = 0;
int redLane = 0;
int lastRedLane = 0;
int redPos;

int greenLap = 0;
int greenLane = 0;
int lastGreenLane = 0;
int greenPos;

int yellowLap = 0;
int yellowLane = 0;
int lastYellowLane = 0;
int yellowPos;

int blueLap = 0;
int blueLane = 0;
int lastBlueLane = 0;
int bluePos;

void setup()
{
  lcd.init();
  lcd.backlight();
  lcd.clear();
  pinMode(8, INPUT_PULLUP);
  pinMode(9, INPUT_PULLUP);
  pinMode(10, INPUT_PULLUP);
  pinMode(11, INPUT_PULLUP);
}

void loop()
{
  redLane = digitalRead(8);
  greenLane = digitalRead(9);
  yellowLane = digitalRead(10);
  blueLane = digitalRead(11);

  if (redLane != lastRedLane)
  {
    if (redLane == LOW)redLaneCount();
  }
  lastRedLane = redLane;
  
  if (greenLane != lastGreenLane)
  {
    if (greenLane == LOW)greenLaneCount();
  }
  lastGreenLane = greenLane;

  if (yellowLane != lastYellowLane)
  {
    if (yellowLane == LOW)yellowLaneCount();
  }
  lastYellowLane = yellowLane;

  if (blueLane != lastBlueLane)
  {
    if (blueLane == LOW)blueLaneCount();
  }
  lastBlueLane = blueLane;
}

void redLaneCount()
{
  lcd.setCursor(16,0);
  lcd.print(redLap);
  redLap++;
  updatePosition();
}
void greenLaneCount()
{
  lcd.setCursor(16,1);
  lcd.print(greenLap);
  greenLap++;
  updatePosition();
}
void yellowLaneCount()
{
  lcd.setCursor(16,2);
  lcd.print(yellowLap);
  yellowLap++;
  updatePosition();
}
void blueLaneCount()
{
  lcd.setCursor(16,3);
  lcd.print(blueLap);
  blueLap++;
  updatePosition();
}

void updatePosition()
{
  int maximum = max(max(redLap, greenLap), max(yellowLap, blueLap));
  int minimum = min(min(redLap, greenLap), min(yellowLap, blueLap));

  int red = map(redLap, maximum, minimum, 0, 3);
  redPos = red+1;
  int green = map(greenLap, maximum, minimum, 0, 3);
  greenPos = green+1;
  int yellow = map(yellowLap, maximum, minimum, 0, 3);
  yellowPos = yellow+1;
  int blue = map(blueLap, maximum, minimum, 0, 3);
  bluePos = blue+1;

  lcd.setCursor(0,0);
  lcd.print("P");
  lcd.setCursor(1,0);
  lcd.print(redPos);
  lcd.setCursor(0,1);
  lcd.print("P");
  lcd.setCursor(1,1);
  lcd.print(greenPos);
  lcd.setCursor(0,2);
  lcd.print("P");
  lcd.setCursor(1,2);
  lcd.print(yellowPos);
  lcd.setCursor(0,3);
  lcd.print("P");
  lcd.setCursor(1,3);
  lcd.print(bluePos);
}

And yes, before anyone ask.. I did try moving the "updatePosition " content inside the "void loop" .. the result is the same..

Thanks again

#10

You know the scene in Amadeus where the emperor tells Mozart "Too many notes" ?

Well, too many words.

#11

Your sorting method is not going to work. The lowest number will map to 0 and the highest number will map to 3 but the two in-between values can map to any of the four numbers.

If you want to sort four numbers it isn't hard. A quick-and-dirty bubble sort will do. It would be easier if you put your four numbers into an array.

It looks like you are trying to determine the order of the four colors. Is that right? Hard to tell with only a small fraction of the code.

#12

Hi, yes that is correct.

Thank you,
I will have a look into it.

I did post a complete code somewhere, should be in one of the replies of this topic..

Thanks again.

#13 
#include <LiquidCrystal_I2C.h>
#include <Wire.h>
LiquidCrystal_I2C lcd(0x3F, 20, 4);

const byte LaneCount = 4;
const char *LaneNames[LaneCount] = {"Red", "Green", "Yellow", "Blue"};
const byte LaneSensorPins[LaneCount] = {8, 9, 10, 11};

// Global variables default to zero
boolean LanePreviousState[LaneCount];
int LapCounter[LaneCount];
unsigned long TimeOfLastLap[LaneCount];
int LanePosition[LaneCount];


void setup()
{
  lcd.init();
  lcd.backlight();
  lcd.clear();

  for (int lane = 0; lane < LaneCount; lane++)
    pinMode(LaneSensorPins[lane], INPUT_PULLUP);
}

void loop()
{
  boolean newData = false;  // Set when a lap crossing happens

  for (int lane = 0; lane < LaneCount; lane++)
  {
    boolean laneState = digitalRead(LaneSensorPins[lane]) == LOW;

    if (laneState != LanePreviousState[lane])
    {
      LanePreviousState[lane] = laneState;

      if (laneState)  // Just went active
      {
        LapCounter[lane]++;
        TimeOfLastLap[lane] = millis();
        newData = true;

        lcd.setCursor(16, lane);
        lcd.print(LapCounter[lane]);
      }
    }
  }

  // If nobody finished a lap, we're done
  if (!newData)
    return;

  // Determine the positions.
  // Compare each lane with the other lanes to count how many are ahead of them
  for (int lane = 0; lane < LaneCount; lane++)
  {
    int aheadOfMe = 0;
    for (int otherLane = 0; otherLane < LaneCount; otherLane++)
    {
      // Skip comparing a lane to itself
      if (lane == otherLane)
        continue;

      // If they have gone more laps, they are ahead of me
      if (LapCounter[lane] > LapCounter[otherLane])
      {
        aheadOfMe++;
        continue;
      }
      else
        // If they have gone the same number of laps but
        // they got there first, they're ahead of me.
        if (LapCounter[lane] == LapCounter[otherLane] &&
            TimeOfLastLap[lane] > TimeOfLastLap[otherLane])
        {
          aheadOfMe++;
          continue;
        }
    }

    // Compared against all other lanes so 'aheadOfMe gives my position
    LanePosition[lane] = aheadOfMe;

    // Display the position for this lane
    lcd.setCursor(0, lane);
    lcd.print("P");
    lcd.setCursor(1, lane);
    lcd.print(LanePosition[lane]);
  }
}
1 Like
#14

Hi John, very elegant code!
Even sorted tiebreaks, excellent!

Thanks a lot! This will definitely get me going..

I did a couple of tweaks so it would display the way I intended

I am going to post the edited lines of code below in case someone wants to do the same.

// If they have gone more laps, they are ahead of me
      if (LapCounter[lane] < LapCounter[otherLane]) //changed from '>' to '<' as it was ranking in the opposite order
      {
        aheadOfMe++;
        continue;
      }

//Also added a +1 to compensate as it would display P0  instead of P1 etc..

// Display the position for this lane
    lcd.setCursor(0, lane);
    lcd.print("P");
    lcd.setCursor(1, lane);
    lcd.print(LanePosition[lane] +1); // <= '+1'

Cheers!

#15

I would add an array of place names:
const char * PlaceNames[LaneCount] = {"1st", "2nd", "3rd", "4th"};

Then you can display the names instead of a number:
lcd.print(PlaceNames[LanePosition[lane]]);

#16

Nice one!

//Just repurposed this line:
//const char *LaneNames[LaneCount] = {"Red", "Green", "Yellow", "Blue"};
const char *PlaceNames[LaneCount] = {"P1", "P2", "P3", "P4"};

and changed the LCD line as suggested, it's working great!

This is an old project I've decided to resurface out of my box shame, I was stuck with the raking logic for a while and left it in the box for about 3-4 years?

It's the first time I've ever asked for help in terms of coding and I wish I did it before..
Thanks a lot John!