Go Down

Topic: Constructor parameter issues (Read 901 times) previous topic - next topic

ichris93

Hello everyone,

I am working on a pretty simple library for switching things on and off.  In my constructor I have included a parameter so it knows if on is HIGH or if on should be LOW.  However, the arduino sketch seems to ignore that and treats it all the the same.  Any assistance would be greatly appreciated.

.h file
Code: [Select]


class Relay
{
  public:
    Relay(int pin, bool kind); //kind 1 is positive on kind 2 is negative on
    void on();
    void off();
    bool kind;
  private:
    int _pin;
};


.cpp file
Code: [Select]

Relay::Relay(int pin, bool kind)
{
  pinMode(pin, OUTPUT);
  _pin = pin;
 
}

void Relay::on(){
if (kind=true){
digitalWrite(_pin, HIGH);
}
else if (kind=false) {
  digitalWrite(_pin, LOW);
  }

}

void Relay::off(){
if (kind=true) {
  digitalWrite(_pin, LOW);
  }
else if (kind=false){
digitalWrite(_pin, HIGH);
}

}


Thanks again!

Coding Badly

Code: [Select]
Relay::Relay(int pin, bool kind)
{
  pinMode(pin, OUTPUT);


Do not call Arduino functions in constructors.  Use a begin method for initializing the I/O.

Coding Badly

Code: [Select]
if (kind=true){
...
else if (kind=false) {


Comparison versus assignment.

majenko


Code: [Select]
Relay::Relay(int pin, bool kind)
{
  pinMode(pin, OUTPUT);


Do not call Arduino functions in constructors.  Use a begin method for initializing the I/O.


To elaborate...

The constructor is called before the main() function which does all the configuration and initialisation of the Arduino.

Anything you put in a constructor has a chance (depending on what it is) of being either over-written by defaults set in main(), or of not doing anything at all because it relies on initialisation that hasn't been performed yet.

Anything which is "arduinoesque", i.e., anything like pinMode(), digitalWrite() etc will deffinately not work in a constructor.  The loosely agreed standard is to provide a .begin() method which you call from setup() to configure any IO etc that needs to be configured.  You can either move your setting parameters to that method, or keep them in the constructor but store them in properties for the .begin() method to use later.
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

ichris93



Code: [Select]
Relay::Relay(int pin, bool kind)
{
  pinMode(pin, OUTPUT);


Do not call Arduino functions in constructors.  Use a begin method for initializing the I/O.


To elaborate...

The constructor is called before the main() function which does all the configuration and initialisation of the Arduino.

Anything you put in a constructor has a chance (depending on what it is) of being either over-written by defaults set in main(), or of not doing anything at all because it relies on initialisation that hasn't been performed yet.

Anything which is "arduinoesque", i.e., anything like pinMode(), digitalWrite() etc will deffinately not work in a constructor.  The loosely agreed standard is to provide a .begin() method which you call from setup() to configure any IO etc that needs to be configured.  You can either move your setting parameters to that method, or keep them in the constructor but store them in properties for the .begin() method to use later.


Thank you for the elaboration.  What do you put in the constructor then? 

ichris93


Code: [Select]
if (kind=true){
...
else if (kind=false) {


Comparison versus assignment.


I'm not sure what you mean. 

Professor Chaos



The constructor is called before the main() function which does all the configuration and initialisation of the Arduino.



Is this always the case?  I.e., if you declare a variable in loop() or setup()?  Or in a function only called by them?

majenko




The constructor is called before the main() function which does all the configuration and initialisation of the Arduino.



Is this always the case?  I.e., if you declare a variable in loop() or setup()?  Or in a function only called by them?


Local variables are constructed and placed on the stack at the time of function call.  So it should work with a local variable.  If the variable is global, then it will be called before main().
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

ichris93

This is what I have changed it to and now it doesn't work at all.
.h
Code: [Select]


class Relay
{
  public:
    Relay(int pin, bool kind); //kind 1 is positive on kind 2 is negative on
    void begin();
    void on();
    void off();
    int pin;
    bool kind;
  private:
   
};

.cpp
Code: [Select]

Relay::Relay(int pin, bool kind)
{
}

void Relay::begin(){
pinMode(pin, OUTPUT);
}

void Relay::on(){
if (kind=true){
digitalWrite(pin, HIGH);
}
else if (kind=false) {
  digitalWrite(pin, LOW);
  }

}

void Relay::off(){
if (kind=true) {
  digitalWrite(pin, LOW);
  }
else if (kind=false){
digitalWrite(pin, HIGH);
}

}



majenko

What are you setting theObject.pin to?  You need to assign it in the constructor:
Code: [Select]

this->pin = pin;

... and the same with "kind".  You're just throwing away those passed parameters.
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

ichris93


What are you setting theObject.pin to?  You need to assign it in the constructor:
Code: [Select]

this->pin = pin;

... and the same with "kind".  You're just throwing away those passed parameters.


Of course, thank you.  I got that working again.  But it still seems to be ignoring my "kind" parameter and treating the pin the same regardless. 

majenko



What are you setting theObject.pin to?  You need to assign it in the constructor:
Code: [Select]

this->pin = pin;

... and the same with "kind".  You're just throwing away those passed parameters.


Of course, thank you.  I got that working again.  But it still seems to be ignoring my "kind" parameter and treating the pin the same regardless. 

That was addressed in an earlier reply:

Code: [Select]

if (kind=true) {

is wrong - it should be == not =.
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

ichris93




What are you setting theObject.pin to?  You need to assign it in the constructor:
Code: [Select]

this->pin = pin;

... and the same with "kind".  You're just throwing away those passed parameters.


Of course, thank you.  I got that working again.  But it still seems to be ignoring my "kind" parameter and treating the pin the same regardless. 

That was addressed in an earlier reply:

Code: [Select]

if (kind=true) {

is wrong - it should be == not =.


The comparison vs. assignment one?  I didn't understand that, but now it is working.  Thank you again for your help.

majenko

= assigns a value, == compares values.

eg:

Code: [Select]

if (a = 1) {


will always be true.  C has a useful function of "value fallthrough", whereby a value being assigned to a variable can be assigned to a second variable at the same time:

Code: [Select]

a = b = 1;

will set both a and b to 1.  One effect of this is that using = in an "if" causes the assignment to fall through to the if, so it assigns 1 to a (for example), and also passes 1 to the if, so it is like writing:

Code: [Select]

a = 1;
if (1) {


Other languages, like BASIC, don't have the distinction between = and ==, and assume that = in an "if" is a comparison.  But then BASIC doesn't have assignment fallthrough, and a huge number of other incredibly powerful facilities like C does ;)
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

dc42

For the code in the .cpp file, this is simpler and probably more efficient:

Code: [Select]

Relay::Relay(int _pin, bool _kind) : pin(_pin), kind(_kind)
{
}

void Relay::begin(){
pinMode(pin, OUTPUT);
}

void Relay::on(){
digitalWrite(pin, (kind) ? HIGH : LOW);
}

void Relay::off(){
  digitalWrite(pin, (kind) ? LOW : HIGH);
}
Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Go Up