Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mux.read(..) apparently doesn't work in setup() #7

Open
fernando-inf opened this issue Sep 1, 2021 · 4 comments
Open

mux.read(..) apparently doesn't work in setup() #7

fernando-inf opened this issue Sep 1, 2021 · 4 comments

Comments

@fernando-inf
Copy link

fernando-inf commented Sep 1, 2021

Hello, in order to simplify my code (to later use the Bounce2 library) I try to create an array of this type before void setup:

int buttonPin [] = {mux.read (0), mux.read (1), mux.read (2), mux.read (3), ... }

However, renaming mux.read (i) only works if it is done inside the void loop.

Am I right or am I doing something wrong?

(I´m not a developer, I´m starting on this. )

@fernando-inf
Copy link
Author

fernando-inf commented Sep 1, 2021

This is the code whit Bounce2, the compilation is fine, but it doesn't work :


#include "Mux.h"
using namespace admux;
Mux mux1(Pin(17, OUTPUT,       PinType::Digital), Pinset(14, 15, 16));
Mux mux2(Pin(18, INPUT_PULLUP, PinType::Digital), Pinset(14, 15, 16));

int buttonPin[]= {mux2.read(0), mux2.read(1), mux2.read(2), mux2.read(3)};
#include <Bounce2.h>//https://github.com/thomasfredericks/Bounce2
Bounce2::Button Pin0 = Bounce2::Button();


void setup() 
              { 
                Pin0.attach(buttonPin[0]);  Pin0.interval(25);  Pin0.setPressedState(LOW);
              
              }

void loop() 
              {
                Pin0.update();  
                if (Pin0.pressed()) {mux1.write(HIGH, 0);} else {mux1.write(LOW, 0);}
              }

Placing in void setup, it doesn't work either:

Pin0.attach(mux2.read(0)); Pin0.interval(25); Pin0.setPressedState(LOW);

@stechio
Copy link
Owner

stechio commented Sep 4, 2021

Hi Fernando,
thanks for using this library.

Before delving into your problem, I noticed some issues about your code that are better to get preliminarily fixed:

  1. bad coding practices
    Coding effectively is a matter of tidiness (you will unavoidably understand it sooner or later during your development process, depending on the seriousness of your commitment). You should strive to properly format your code lines/blocks and appropriately name your entities (variables, functions, types, and so on) -- there are tons of resources out there to guide you to the best practices. I may sound pedantic, but abiding by the hard-learned suggestions from experienced developers will dramatically improve your coding life and ease your communication with other developers.
  2. Arduino life cycle
    As hardware is initialized only AFTER the global declarations, global variables CANNOT be initialized accessing I/O (it will be available only from setup() invocation on).

Here it is your code normalized according to my suggestions:

/*
include directives should be typically kept at the top of the source file
*/
#include "Mux.h"
#include <Bounce2.h>//https://github.com/thomasfredericks/Bounce2

using namespace admux;

/*
Constants should be fully upper-cased.
*/
const int BUTTON_PINS_COUNT = 4;

/*
`mux1` and `mux2` are BAD variable names:
- their purposes should be obvious to the human reader (instead, `mux1` and
`mux2` are obscure (what are they supposed to do?)), so I renamed them to
more meaningful `outMux` and `inMux` (change them according to your intents).
*/
Mux outMux(Pin(17, OUTPUT, PinType::Digital), Pinset(14, 15, 16));
Mux inMux(Pin(18, INPUT_PULLUP, PinType::Digital), Pinset(14, 15, 16));

/*
You cannot execute I/O before the setup() function gets invoked, as here
(declaration level) the hardware hasn't been initialized yet. Therefore, `buttonPin`
can be declared but NOT assigned with mux.read(..) results.
Since it is an array, I would suggest to name it as plural (`buttonPins`) to make it
apparent.
*/
// int buttonPin[]= {mux2.read(0), mux2.read(1), mux2.read(2), mux2.read(3)};
int buttonPins[BUTTON_PINS_COUNT];

/*
`Pin0` is a BAD variable name:
- initial letter should be low-cased;
- its purpose should be obvious to the human reader (instead, "pin0" is obscure
(what is it supposed to do?)), so I renamed it to a more meaningful `button` (change
it according to your intents).
*/
Bounce2::Button button = Bounce2::Button();

void setup() {
    for (int i = 0; i < BUTTON_PINS_COUNT; i++) {
        buttonPins[i] = inMux.read(i);
    }
    
    button.attach(buttonPins[0]); 
    button.interval(25); 
    button.setPressedState(LOW);
}

void loop() {
    button.update();
    if (button.pressed()) {
        outMux.write(HIGH, 0);
    } else {
        outMux.write(LOW, 0);
    }
}

Coming to your issue, there's another practice I would suggest: whenever a seemingly obscure problem occurs, follow the golden strategy of software development: Divide and Conquer. Isolate the problem, stripping your code down to its bare minimum that keeps reproducing the same faulty behavior. Once you have solved it, retry your full code. If other issues emerge, repeat the strip/solve cycle until all is fixed.

In your case, I would first try to execute mux.read(0) by itself, without any of the Bounce2-related stuff:

#include "Mux.h"

using namespace admux;

Mux inMux(Pin(18, INPUT_PULLUP, PinType::Digital), Pinset(14, 15, 16));

int initialData;

void setup() {
    initialData = inMux.read(0);
}

void loop() {
  Serial.print("Current value: "); Serial.println(inMux.read(0));
  Serial.print("Initial value: "); Serial.println(initialData);
  Serial.println();

  delay(1000);
}

Does it work for you this way? I've just got a try by myself reading a simple button through an 8-channel 4051 mux and it worked as expected even in the setup() function:

#include "Mux.h"

using namespace admux;

/*
 * 4051 mux wired to a NodeMCU.
 */
Mux mux(Pin(D4, INPUT_PULLUP, PinType::Digital), Pinset(D1, D2, D3));

int initialData;

void setup() {
  Serial.begin(9600);

  initialData = mux.read(0);
}

void loop() {
  Serial.print("Button value: "); Serial.println(mux.read(0));
  Serial.print("Button initial value: "); Serial.println(initialData);
  Serial.println();

  delay(1000);
}

BTW, what kind of MCU and mux are you using?

@stechio stechio changed the title Array whit mux.read (i) mux.read(..) apparently doesn't work in setup() Sep 4, 2021
@fernando-inf
Copy link
Author

Hello, I appreciate your prompt response and your corrections. I am already correcting my code based on your suggestions.

On the other hand, right now I am only simulating the Mux 4051 in SimulIDE, with Arduino Nano. Library "arduino-ad-mux-lib" without using Bounce2 works fine. But, your proposal implementing the use of Bounce2 is, in short, the same as the last line of my second comment:

Pin0.attach (mux2.read (0)); 
Pin0.interval (25); 
Pin0.setPressedState (LOW);

and as I said it does not work, at least in SimulIDE. When I have more time I will try to test the function physically.

P.D: I think the best test is doing it with more than one button, since connecting more than one mux in the same S0, S1, S2, can cause synchronization problems, when you want to turn on LEDs with buttons and keep them on.

@fernando-inf
Copy link
Author

fernando-inf commented Sep 5, 2021

Something that has occurred to me, following the example of my code, assign pin 18 of Arduino to attach (bounce2):

void setup () {
  button.attach (18);
  button.interval (25);
  button.setPressedState (LOW);
}

Later:


void loop () {
  button.update ();
  if (button.isPressed ()) {
    outMux.write (HIGH, 0);
  } else {
    outMux.write (LOW, 0);
  }
}

This works in SimulIDE but only if bounce2's isPressed function and a push button are used. When the number of pushbuttons and LEDs is increased, the errors begin. This is because I can't find the way to make the connection of each mux pin (inMux.read (0), inMux.read (1); ...) correctly when pressing the button, it occurred to me:


void loop () {
  button.update ();

  inMux.read (0);
  if (button.isPressed ()) {
    outMux.write (HIGH, 0);
  } else {
    outMux.write (LOW, 0);
  }
  delay (10);

  inMux.read (1);
  if (button.isPressed ()) {
    outMux.write (HIGH, 1);
  } else {
    outMux.write (LOW, 1);
  }
  delay (10);
}

But it doesn't work, since pressing the first button does nothing, and when pressing the second the first and second LEDs turn on at the same time.

P.D: My language is not English, sorry for the bad expressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants