Unreliable ranking values using switch case

Frandabenta · May 14, 2021, 7:00pm

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..

anon73444976 · May 14, 2021, 7:04pm

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

Frandabenta · May 14, 2021, 7:14pm

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.

Railroader · May 14, 2021, 7:14pm

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.

sterretje · May 14, 2021, 7:22pm

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

redpos = red+1;

Frandabenta · May 14, 2021, 7:40pm

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

I will keep on trying

sterretje · May 14, 2021, 7:47pm

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.

anon73444976 · May 14, 2021, 7:47pm

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?

Frandabenta · May 14, 2021, 8:00pm

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

anon73444976 · May 14, 2021, 8:05pm

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

Well, too many words.

johnwasser · May 14, 2021, 10:38pm

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.

Frandabenta · May 14, 2021, 10:57pm

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.

johnwasser · May 15, 2021, 11:35am

#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]);
  }
}

Frandabenta · May 15, 2021, 12:28pm

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!

johnwasser · May 15, 2021, 12:40pm

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]]);

Frandabenta · May 15, 2021, 1:00pm

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!

system · September 12, 2021, 1:00pm

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