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

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!

Please post a program illustrating the problem

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

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.

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?

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

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.

Thanks guys! Stuff to play with :slight_smile:

Marked solved