[SOLVED] digitalWrite or pinMode not functioning inside .cpp file

Projects Programming
1

Good afternoon! Recently I've moved a functioning sketch into it's own library & class, and now upon testing 'er out as a new fancy organized library, none of my test LEDs appear to be turning on :o

Anybody have any experience with this?

Relevant code:
.h file

#ifndef global_Conv_h
#define global_Conv_h

#include "Arduino.h"
#include "ARD1939.h"

class Conv {
 public:
 Conv(byte ledPinGreen, byte ledPinEnable);
 void Startup();
 void LineStatus(byte &nMsgId, long &lPGN, byte pMsg[], int &nMsgLen, byte &nDestAddr, byte &nSrcAddr, byte &nPriority);
 void SrcAddr(byte &nSrcAddr);
 void SendHello(byte nSrcAddr, int &nCounter);
 void ParseJ1939(byte &nMsgId, int &nMsgLen, long &lPGN, byte pMsg[], byte nDestAddr, byte nSrcAddr, byte nPriority);
 void MotorEnabled();
 void ECUSeen();
 void OutputVMotor(short RPM);
 void CommsTimeout();
 void SerialPrint(long &lPGN, byte nDestAddr, byte nSrcAddr, byte nPriority, int &nMsgLen, byte pMsg[]);
 byte _ledPinGreen;
 byte _ledPinEnable;
 byte nJ1939Status;
 bool boolRun;
 unsigned long nCount1;
 ARD1939 j1939;
};

#endif

.cpp file

#include "global_Conv.h"

Conv::Conv(byte ledPinGreen, byte ledPinEnable){
 //Configure pins for output & input. 13 & 12
 const byte _ledPinGreen = ledPinGreen;
 const byte _ledPinEnable = ledPinEnable;
[b]//I have tried putting the pinMode here as well[/b]
}
 
void Conv::Startup(){
 pinMode(_ledPinGreen, OUTPUT);
 pinMode(_ledPinEnable, OUTPUT);
 //establish J1939 communications via ARD1939 library.
 // Set the serial interface baud rate
 Serial.begin(MONITOR_BAUD_RATE);

 // Initialize the J1939 protocol including CAN settings
 if(j1939.Init(SYSTEM_TIME) == CAN_OK)
 Serial.print("CAN Controller Init OK.\n\r\n\r");
 else
 Serial.print("CAN Controller Init Failed.\n\r");
    
 // Set the preferred address and address range
 j1939.SetPreferredAddress(SA_PREFERRED);
 j1939.SetAddressRange(ADDRESSRANGEBOTTOM, ADDRESSRANGETOP);

 // Set the message filter
 j1939.SetMessageFilter(65280);
 j1939.SetMessageFilter(65281);
 //Inverter inputs
 j1939.SetMessageFilter(65297); //Inverter: 1.4.1.1
 j1939.SetMessageFilter(65313); //Inverter: 1.4.1.2
 j1939.SetMessageFilter(65314); //Inverter: 1.4.1.3
 j1939.SetMessageFilter(65315); //Inverter: 1.4.1.4
 j1939.SetMessageFilter(65316); //Inverter: 1.4.1.5
 j1939.SetMessageFilter(65317); //Inverter: 1.4.1.6
 j1939.SetMessageFilter(65318); //Inverter: 1.4.1.7
 j1939.SetMessageFilter(65319); //Inverter: 1.4.1.8

 // Set the NAME
 j1939.SetNAME(NAME_IDENTITY_NUMBER,
 NAME_MANUFACTURER_CODE,
 NAME_FUNCTION_INSTANCE,
 NAME_ECU_INSTANCE,
 NAME_FUNCTION,
 NAME_VEHICLE_SYSTEM,
 NAME_VEHICLE_SYSTEM_INSTANCE,
 NAME_INDUSTRY_GROUP,
 NAME_ARBITRARY_ADDRESS_CAPABLE);
 
}

void Conv::ParseJ1939(byte &nMsgId, int &nMsgLen, long &lPGN, byte pMsg[], byte nDestAddr, byte nSrcAddr, byte nPriority){
 // Check for a received message
 if(nMsgId == J1939_MSG_APP){
 SerialPrint(lPGN, nDestAddr, nSrcAddr, nPriority, nMsgLen, pMsg); [b]//This works[/b]
 for(byte cIndex = 0; cIndex < nMsgLen; cIndex++){ //What kind of message was received
 if ((cIndex == 7) &&
 (lPGN == 65297) &&
 (pMsg[0] == 0x38) &&
 (pMsg[1] == 0x37) &&
 (pMsg[2] == 0x36) &&
 (pMsg[3] == 0x35) &&
 (pMsg[4] == 0x88) &&
 (pMsg[5] == 0x88) &&
 (pMsg[6] == 0x88) &&
 (pMsg[7] == 0x88)) {
 digitalWrite(_ledPinGreen, HIGH); [b]//This does not work[/b]
 ECUSeen();
 }
 }
 }
}

void Conv::MotorEnabled(){
 boolRun = true;
 nCount1 = millis();
}

void Conv::ECUSeen(){
 digitalWrite(_ledPinGreen, HIGH);
}

void Conv::SerialPrint(long &lPGN, byte nDestAddr, byte nSrcAddr, byte nPriority, int &nMsgLen, byte pMsg[]){
 Serial.print("PGN:");
    Serial.print(lPGN, HEX);

    Serial.print(" DA:");
    Serial.print(nDestAddr);

    Serial.print(" SA:");
    Serial.print(nSrcAddr);

    Serial.print(" P:");
    Serial.print(nPriority);

    Serial.print(" Data:");
 for(byte cIndex = 0; cIndex < nMsgLen; cIndex++){
      Serial.print(pMsg[cIndex], HEX);
      Serial.print(" ");
 }
 Serial.print("\n\r");
}

A few more details: the Serial.print shows that pMsg is indeed coming in with the data being looked for (0x38, 0x37, ...), however the digitalWrite isn't doing anything :0

Insight is much appreciated!

2

Please post a program illustrating the problem

3

My apologies, here is the sketch:

#include "ID.h" //J1939 ID

#include <stdlib.h>
#include <inttypes.h>

#include "ARD1939.h"

int nCounter = 0;

#include "global_Conv.h" //Converter Library

Conv LC(13, 12);

void setup(){
	LC.Startup();
}

void loop(){
	// J1939 Variables
	byte nMsgId; //Ideally, put this stuff in the library...
	byte nDestAddr;
	byte nSrcAddr;
	byte nPriority;
	byte nJ1939Status;
	int nMsgLen;
	long lPGN;
	byte pMsg[J1939_MSGLEN];

	// Establish the timer base in units of milliseconds
	delay(SYSTEM_TIME);//This slows the Arduino down! Is it necessary for the J1939 timings?
	// Call the J1939 protocol stack
	LC.LineStatus(nMsgId, lPGN, pMsg, nMsgLen, nDestAddr, nSrcAddr, nPriority);
	LC.SrcAddr(nSrcAddr);
	LC.SendHello(nSrcAddr,nCounter);
	LC.ParseJ1939(nMsgId, nMsgLen, lPGN, pMsg, nDestAddr, nSrcAddr, nPriority);
}// end loop
4

Change

Conv::Conv(byte ledPinGreen, byte ledPinEnable){
  const byte _ledPinGreen = ledPinGreen;
  const byte _ledPinEnable = ledPinEnable;
}

to

Conv::Conv(byte ledPinGreen, byte ledPinEnable){
  _ledPinGreen = ledPinGreen;
  _ledPinEnable = ledPinEnable;
}

or

Conv::Conv(byte ledPinGreen, byte ledPinEnable) : 
  _ledPinGreen(ledPinGreen), _ledPinEnable(ledPinEnable) {}

your original code does not set _ledPinGreen and _ledPinEnable.

5

Aaaaah I must've done that when I tried making stuff "const" to conserve space and ran into compiler errors. Good eye, thank you!

Follow-up, if you could be so kind: if I wanted to make a variable ("_ledPinGreen") be const within the class, would I declare it in the constructor? Or would this typically just be forgone in favor of simply declaring it normally?

6

If you define it in the constructor, then it is only visible in the constructor; not what you want.

If you define it as "const" in the private part of the class, then you must use the second method @Whandall showed

7

If you want it to be const, you should declare it const in the class definition
and use the second variant of the given initialization which handles const.

8

Thanks guys! Stuff to play with :slight_smile:

Marked solved