Pages: [1]   Go Down
Author Topic: Constructor parameter issues  (Read 658 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:

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:
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!
Logged

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 176
Posts: 12283
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
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.
Logged

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 176
Posts: 12283
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Comparison versus assignment.
Logged

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
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.
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
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? 
Logged

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Comparison versus assignment.

I'm not sure what you mean. 
Logged

0
Offline Offline
Full Member
***
Karma: 1
Posts: 225
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


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?
Logged

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset


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().
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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:
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);
}

}

Logged

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

What are you setting theObject.pin to?  You need to assign it in the constructor:
Code:
this->pin = pin;
... and the same with "kind".  You're just throwing away those passed parameters.
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

What are you setting theObject.pin to?  You need to assign it in the constructor:
Code:
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. 
Logged

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

What are you setting theObject.pin to?  You need to assign it in the constructor:
Code:
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:
if (kind=true) {
is wrong - it should be == not =.
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

What are you setting theObject.pin to?  You need to assign it in the constructor:
Code:
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:
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.
Logged

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

= assigns a value, == compares values.

eg:

Code:
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:
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:
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 smiley-wink
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

United Kingdom
Offline Offline
Tesla Member
***
Karma: 220
Posts: 6587
Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Code:
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);
}
Logged

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.

Pages: [1]   Go Up
Jump to: