Too many arguments to function 'void Lights(int)'

Projects Programming
1 
void Blink(int lightColour, int frequency)
{
  frequency = 1000/frequency;
  frequency /= 2;

  if(onOff && millis()>=blinkTime+frequency)
  {
    Lights(lightColour);

    onOff = !onOff;
    blinkTime = millis ();
  }

  if(!onOff && millis()>=blinkTime+frequency)
  {
    Lights(0,0,0);

    onOff = !onOff;
    blinkTime = millis();
  }
}

bool Gateposition()
{
  if(ProxSens() && !crossingState)
  {
    LastCarDetectTime = millis();
  }
  if(LastCarDetectTime = 0 || millis()>LastCarDetectTime + C)
  {
    return 0;
  }
  else
  {
    return 1;
  }
}

void Lights(int colour)
{
  switch(colour) {
    case0://red
    Lights(255,0,0);
    break;

    case 1://green
    Lights(0,255,0);
    break;

    case  2:
    Lights(0,0,255);
    break;
  }
}
2

vs

   Lights(0,255,0);

Why this additional 0?

Why a new thread?

3 
    Lights(0, 0, 0);

Whoops !
Lights() only expects a singe int as its parameter

4

Isn't it a little confusing to pass a parameter 'frequency' and then immediately make it the period/2? You should create a new variable 'period' for that, or choose a descriptive name you like.

5

Almost any time you see repetition you can factor the code to make it smaller and easier to follow. Also an 'if' followed by an 'if not' is what 'if else' was invented for:

if (millis() >= blinkTime + frequency) {
  if (onOff) {
    Lights(lightColour);
  }
  else {
    Lights(Black);
  }
  onOff = !onOff;
  blinkTime = millis();
}
6

missing function overload that takes 3 ints

7

The problem is mainly that the function doesn't have a case for "black", only red green and blue. You can't turn the lights off without that (I'm assuming that is what 0,0,0 is supposed to do). So you should

void Lights(int colour)
{
  switch(colour) {
    case0://red
    Lights(255,0,0);
    break;

    case 1://green
    Lights(0,255,0);
    break;

    case  2: //blue, apparently
    Lights(0,0,255);
    break;

    case  3:  // off (black)
    Lights(0,0,0);
    break;
  }
}
8 
void Lights(int colour)
{
  switch(colour) {
    case0://red
    Lights(255,0,0);
    break;

I'm looking at that and all I can see is something that won't compile because there is no three-argument Lights() function.

So maybe

void Lights(int colour)
{
  switch(colour) {
    case0://red
    setColour(255,0,0);
    break;

at which point a setColor() taking a traditional 32 bit color integer could be written, at which point Lights() can use an index into an array of RGB colours.

Or not.

a7

9

Any suggestions here, including mine, will be sub optimal because the OP did not post full code.

A lot of similar functions break out the blink as a parameter, e.g.

void setLED(colour_t colour, bool blinkStatus) {

Just a thought... you only call this when you want to set or change a colour/blink state...

With a separate service routine to handle the blink:

void updateLED() {

Then use stone age global variables shared between the two routines, if a class is considered too complex.

10

@anon57585045 Indeed. Normally annoying, just plain frustrating here. I visited the other thread to find a bit more to go on.

a7

11

he exceeded the limit of answers in the other thread, so he can't post there today.

12

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