Mathematical or variable type problem

I have a sketch that monitors inputs. When inputs are pulled low, elements in the array change from 0 to 1.
Two elements change for respective input changes so
input_0 is pulled low, a 1 is written to elements 0 and 1
input_1 is pulled low, a 1 is written to elements 2 and 3
etc
And the array is then written to the serial monitor.

max is the maximum number of inputs that will need to be checked.
It all works fine, as long as max < 9
For max >=8 there is no output to the serial monitor.

I’ve narrowed the problem down to the two commented lines.
I wonder whether it’s related to max being a byte (8 bits) but I expect that to be upto 255, not 8. And changing all the bytes to int doesn’t make any difference.
I’ve also tried different mathematical operations (rather than multiplying by 2) and that didn’t fix it either.

What is the really simple problem, and what really simple thing needs to change?

(Thanks in advance.)

const byte max = 8;      //Serial monitor only prints values for max<9
const byte checkPin[max] = {2, 3, 4, 5};

void setup() 
{
  Serial.begin(9600);
  for (int i=0; i<max; i++)
  {
    pinMode(checkPin[i], INPUT_PULLUP);
  }
}

void loop() 
{
  bool connected[max];
  int count = 0;

  for (int i=0; i<max; i++)
  {
    int j=2*i;
    connected[j] = !digitalRead(checkPin[i]);
    j=j+1;                         // This opperation appears to be part of the problem.
    connected[j] = !digitalRead(checkPin[i]);
    if (connected[i] == true)
    {
      count = count +1;
    }
  }
  for (int i=0; i<max; i=i+2)
  {
    Serial.print(connected[i]);
    Serial.print(connected[i+1]);
  }
  Serial.print("\t");
  Serial.println(count);
}

Perhaps because your connected array is HALF the size it needs to be?

When i >= 4, what do you expect to be returned in the following call?digitalRead(checkPin[i]);What do you think the value of these bools are?bool connected[max];

If you have warnings turned up it will warn you about using element 8 from an 8-element array:

sketch_dec26a.ino: In function 'loop':
sketch_dec26a.ino:22:18: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
     connected[j] = !digitalRead(checkPin[i]);
     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
sketch_dec26a.ino:19:21: note: within this loop
   for (int i = 0; i < max; i++)
                   ~~^~~~~
/Users/john/Library/Arduino15/packages/arduino/hardware/avr/1.8.3/cores/arduino/main.cpp: In function 'main':


sketch_dec26a.ino:22:18: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
     connected[j] = !digitalRead(checkPin[i]);
                  ^
sketch_dec26a.ino:19:21: note: within this loop
   for (int i = 0; i < max; i++)
                     ^
Sketch uses 2318 bytes (7%) of program storage space. Maximum is 32256 bytes.
Global variables use 198 bytes (9%) of dynamic memory, leaving 1850 bytes for local variables. Maximum is 2048 bytes.

This version corrects the various indexing errors. I tested it with 10 pins and it works for me.

const byte checkPin[] = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11};
const byte max = sizeof checkPin / sizeof checkPin[0];

void setup()
{
  Serial.begin(115200);
  for (int i = 0; i < max; i++)
  {
    pinMode(checkPin[i], INPUT_PULLUP);
  }
}


void loop()
{
  bool connected[max * 2];
  int count = 0;


  for (int i = 0; i < max; i++)
  {
    bool val = !digitalRead(checkPin[i]);
    connected[i * 2] = val;
    connected[i * 2 + 1] = val;
    if (val)
    {
      count += 1;
    }
  }


  for (int i = 0; i < max; i++)
  {
    Serial.print(connected[i * 2]);
    Serial.print(connected[i * 2 + 1]);
  }
  Serial.print("\t");
  Serial.println(count);
}

Thanks all.
Simply fixed by ...

bool connected [2 * max];

John - Thanks for the tip on warnings. I didn't know about that but found it in the IDE preferences.
And your code is a lot more elegant than mine. :slight_smile:

Is there specific benefit to digitalRead into val and then pass that to the array elements rather than directly to them?

I haven't used sizeof() before either. Is there a reason why you've used sizeof checkpin rather than sizeof (checkpin) ?

fenghuang:
I haven't used sizeof() before either. Is there a reason why you've used sizeof checkpin rather than sizeof (checkpin) ?

I am not speaking for John, but sizeof is an operator, not a function, so parenthesis are only needed if casting a variable type. They are often overused harmlessly in the plethora of tutorials, creating the misunderstanding, which could lead to some nasty bugs.

fenghuang:
John - Thanks for the tip on warnings. I didn't know about that but found it in the IDE preferences.
And your code is a lot more elegant than mine. :slight_smile:

Most of that is decades of experience and my wanting to set a reasonably good example.

fenghuang:
Is there specific benefit to digitalRead into val and then pass that to the array elements rather than directly to them?

Like all functions, the digitalRead() can produce a different answer every time you call it. That means two things:

  • the compiler can't optimize any of the calls away
  • If your code ever assumes that both samples are the same, it will fail in the unlikely case that the pin state changed between the samples.

fenghuang:
I haven't used sizeof() before either. Is there a reason why you've used sizeof checkpin rather than sizeof (checkpin) ?

I prefer not to use extra parens on the argument to the 'sizeof' operator to encourage beginners to learn why they're not needed. Similarly for the 'return' statement, which can also take an argument but is not a function.

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