[SOLVED] Passing an array in a function was working, but now it's not?

Hello!
I’m currently making a modification for Moppy2 ( https://github.com/SammyIAm/Moppy2 ) which allows me to have a hub that receives the MoppyMessages from the Java software and sends them to their destination through an I²C bus. More convenient when you have 48 floppy drives and 8 arduinos (more to come)!

I’ve created a hub with the following code :

/* MoppyMessages contain the following bytes:
    0    - START_BYTE (always 0x4d)
    1    - Device address (0x00 for system-wide messages)
    2    - Sub address (Ignored for system-wide messages)
    3    - Size of message body (number of bytes following this one)
    4    - Command byte
    5... - Optional payload
*/
#include <Wire.h>
// Settings for I2C slaves adresses and pong response
#define MIN_ADDRESS 0x01
#define MAX_ADDRESS 0xA
// Definitions for command byte values

#define START_BYTE 0x4d
#define SYSTEM_ADDRESS 0x00

#define NETBYTE_SYS_PING 0x80
#define NETBYTE_SYS_PONG 0x81
#define NETBYTE_SYS_RESET 0xff
#define NETBYTE_SYS_START 0xfa
#define NETBYTE_SYS_STOP 0xfc

#define NETBYTE_DEV_RESET 0x00
#define NETBYTE_DEV_NOTEOFF 0x08
#define NETBYTE_DEV_NOTEON 0x09
#define NETBYTE_DEV_BENDPITCH 0x0e
uint8_t messagePos = 0;
uint8_t messageBuffer[259];
uint8_t pongBytes[8] = {START_BYTE, 0x00, 0x00, 0x04, 0x81, SYSTEM_ADDRESS, MIN_ADDRESS, MAX_ADDRESS}; // Sending the system adress to identify as a hub. Sub adresses represents the adresses on the network.
void setup() {
  Serial.begin(57600);
  Wire.begin(); // join i2c bus (address optional for master)
}

void sendSystemMessage(uint8_t sysCommand, uint8_t payload[]) {
  for (int i = MIN_ADDRESS; i <= MAX_ADDRESS; i++) {
    Wire.beginTransmission(i);
    Wire.write(1);
    Wire.write(sysCommand);
    Wire.write(payload, sizeof(payload));
    Wire.endTransmission();
  }
}
void sendDeviceMessage(uint8_t deviceAdress, uint8_t deviceSubAdress, uint8_t deviceCommand, uint8_t payload[]) {
    Wire.beginTransmission(deviceAdress);
    Wire.write(0);
    Wire.write(deviceSubAdress);
    Wire.write(deviceCommand);
    Wire.write(payload, sizeof(payload));
    Wire.endTransmission();
}
void loop() {
  while ((messagePos != 4 && Serial.available())
         || (messagePos == 4 && Serial.available() >= messageBuffer[3])) {

    switch (messagePos) {
      case 0:
        if (Serial.read() == START_BYTE) {
          messagePos = 1;
        }
        break;
      case 1:
        messageBuffer[1] = Serial.read(); // Read device address
        messagePos = 2;
        break;
      case 2:
        messageBuffer[2] = Serial.read(); // Read sub address
        messagePos++;
        break;
      case 3:
        messageBuffer[3] = Serial.read(); // Read message body size
        messagePos++;
        break;
      case 4:
        // Read command and payload
        Serial.readBytes(messageBuffer + 4, messageBuffer[3]);
        if (messageBuffer[1] == SYSTEM_ADDRESS) {
          if (messageBuffer[4] == NETBYTE_SYS_PING) {
            Serial.write(pongBytes, sizeof(pongBytes)); // Respond with pong if requested
          } else {
            sendSystemMessage(messageBuffer[4], &messageBuffer[5]); // Send the system wide message
          }
        } else {
          sendDeviceMessage(messageBuffer[1], messageBuffer[2], messageBuffer[4], &messageBuffer[5]); // Send the message and the payload with the sub adress to the device
        }
        messagePos = 0; // Start looking for a new message
    }
  }
}

And I’m basically breaking down this file https://github.com/SammyIAm/Moppy2/blob/master/Arduino/Moppy/src/MoppyNetworks/MoppySerial.cpp into two parts
One part that “decodes” the serial output as in the first part of this code and sends it through I2C (the hub part) and the seconde one on all the Moppy clients that receives I2C messages and then call the handler to treat them.
Here’s the modified “MoppySerial.cpp” for I2C reception:

#include "../MoppyNetworks/MoppySerial.h"
#include <Wire.h>
/*
 * Serial communications implementation for Arduino.  Handler
 * functions are called to consume system and device messages received from
 * the network.
 */
MoppySerial::MoppySerial(handleSystemMessage sys, handleDeviceMessage dev) {
  messageBuffer[0] = START_BYTE;
  systemHandler = sys;
  deviceHandler = dev;
}
void receiveEvent(int howMany) {
    uint8_t payload[64];
    uint8_t message; 
      if (Wire.read() == 1) { // if 1, it's a system event
        message = Wire.read();
        byte i = 0;
        while (Wire.available() > 0)
        {
          payload[i] = Wire.read();
          i++;
        }
        systemHandler(message, &payload[0]);
      }
}
void MoppySerial::begin(long baud) {
  Wire.begin(DEVICE_ADDRESS);
  Wire.onReceive(receiveEvent);
  }

/* MoppyMessages contain the following bytes:
 *  0    - START_BYTE (always 0x4d)
 *  1    - Device address (0x00 for system-wide messages)
 *  2    - Sub address (Ignored for system-wide messages)
 *  3    - Size of message body (number of bytes following this one)
 *  4    - Command byte
 *  5... - Optional payload
 */


void MoppySerial::sendPong() {
  // Do nothing.. Pong is currently handled by the hub
}

When compiling, it’s telling me : "error: ‘systemHandler’ was not declared in this scope

systemHandler(message, &payload[0]);
^
"
I understood that it was not possible to pass an array like that in C, but it is done on the original file here and received successfully by the instrument here
What is wrong ?
Thanks in advance for any help :slight_smile:

Maybe:

void MoppySerial::receiveEvent(int howMany) {

I removed the namespace out of this function since I was told that it was not a correct use of a non static function (Wire related problem)

Lothean:
I removed the namespace out of this function since I was told that it was not a correct use of a non static function (Wire related problem)

systemHandler is a (in the original header file) private member of the MoppySerial class. If receiveEvent() wants to access systemHandler directly then it needs to be a member function of MoppySerial and will need the class prefix in its implementation.

Who told you about the incorrect usage? What was the Wire related problem?

Here's the given error after setting the receiveEvent function in the namespace:

C:\Users\Lothean\AppData\Local\Temp\arduino_build_94274\sketch\src\MoppyNetworks\MoppySerial.cpp: In member function 'void MoppySerial::begin(long int)':

C:\Users\Lothean\AppData\Local\Temp\arduino_build_94274\sketch\src\MoppyNetworks\MoppySerial.cpp:29:30: error: invalid use of non-static member function

   Wire.onReceive(receiveEvent);

                              ^

I modified the code like you told me to :

void MoppySerial::receiveEvent(int howMany) {

And declared this function like this in the header file :

class MoppySerial {
  public:
    MoppySerial(handleSystemMessage, handleDeviceMessage);
    void begin(long baud = MOPPY_BAUD_RATE);
	void receiveEvent(int howMany);
  private:
    handleSystemMessage systemHandler;
    handleDeviceMessage deviceHandler;
    uint8_t messagePos = 0; // Track current message read position
    uint8_t messageBuffer[259]; // Max message length for Moppy messages is 259
    uint8_t pongBytes[8] = {START_BYTE, 0x00, 0x00, 0x04, 0x81, DEVICE_ADDRESS, MIN_SUB_ADDRESS, MAX_SUB_ADDRESS};
    void sendPong();
};

I read online that Wire could not access a function under a namespace for the receive event, but it might be wrong and the problem might be somewhere else

OK, I see. Basically the Wire library's Wire.onReceive(handler) has been designed to only take a plain function as a handler/callback.
Since there is normally only one I2C interface on the hardware, the Wire library is designed (loosely) as a singleton (the single Wire instance being declared in the Wire.h header file). As a singleton, it was fair game to only have one plain function callback. This makes your situation a little difficult though.

I'm not familiar with Moppy, but you could work around the problem by accepting that MoppySerial is also a singleton: declare&define a singleton instance, add a static class function, or an function outside the class, to it that simply calls the receiveEvent() using the singleton instance, and then use the static function as the handler.

Maybe there is a better way, someone else may chime in with one.

BTW:

void receiveEvent(int howMany) {
  uint8_t payload[64];

I would be wary about declaring large buffers like this as local variables (ie. on the stack) in a library since the memory usage will not show up in the compile time RAM usage statistics. It increases the risk of a sketch crashing at run time and will confuse a user of the library.

Hello!
Thanks for the advices.
I will move the buffer declaration elsewhere and hope all of these change will not make my 10 ATmega168 struggle!
Call me noob, but I could not understand how to declare MoppySerial as a singleton (tutorials seemed confusing to me as I'm not a native English speaker)
Could you lead me to something that could help me ?

Well, you can look the the header file and source files of the Wire library to see what I mean. You really are just creating an single instance of the class; a proper singleton implementation might enforce the single instance by restricting the constructors of the class and providing a factory function, but it's not necessary.

So something like adding this to the end of your header:

extern MoppySerial TheMoppySerial;

Add the arguments from the constructor to the begin() function.
Change the constructor to take no parameters.

Then in the cpp file:

MoppySerial TheMoppySerial;
void receiveEventHandler(int howMany) {
  TheMoppySerial.receiveEvent(howMany);
}
...
void MoppySerial::begin(...) {
  ...
  Wire.onReceive(receiveEventHandler);
  }
}

Move the parameters stuff from the constructor to the begin().
And whatever else I've forgotten.

The caller of the library then exclusively uses the predefined TheMoppySerial instance.

All this with the caveat that it is a bit ugly and there may be a nicer way of doing things, probably involving a bit of a redesign. But like I say, I'm not familiar with Moppy.

I tried different combinations of code and I’m currently stuck here :
.cpp :

MoppySerial::MoppySerial(handleSystemMessage sys, handleDeviceMessage dev) {
  messageBuffer[0] = START_BYTE;
  systemHandler = sys;
  deviceHandler = dev;
}

MoppySerial TheMoppySerial;
void receiveEventHandler(int howMany) {
  TheMoppySerial.receiveEvent(howMany);
}

void MoppySerial::receiveEvent(int howMany) {
    uint8_t payload[64]; // TO REDECLARE SOMEWHERE ELSE BEFORE FIRST UPLOADING
    uint8_t message; 
      if (Wire.read() == 1) { // if 1, it's a system event
        message = Wire.read();
        byte i = 0;
        while (Wire.available() > 0)
        {
          payload[i] = Wire.read();
          i++;
        }
        systemHandler(message, &payload[0]);
      }
}
void MoppySerial::begin(long baud) {
  Wire.begin(DEVICE_ADDRESS);
  Wire.onReceive(receiveEventHandler);
  }
........

.h :

/*
 * MoppySerial.h
 *
 */

#ifndef SRC_MOPPYNETWORKS_MOPPYSERIAL_H_
#define SRC_MOPPYNETWORKS_MOPPYSERIAL_H_

#include <stdint.h>
#include "Arduino.h"
#include "../../MoppyConfig.h"
#include "MoppyNetwork.h"

#define MOPPY_BAUD_RATE 57600

typedef void (*handleSystemMessage)(uint8_t, uint8_t[]);
typedef void (*handleDeviceMessage)(uint8_t, uint8_t, uint8_t[]);

class MoppySerial {
  public:
    MoppySerial(handleSystemMessage, handleDeviceMessage);
    void begin(long baud = MOPPY_BAUD_RATE);
	void receiveEvent(int howMany);
  private:
    handleSystemMessage systemHandler;
    handleDeviceMessage deviceHandler;
    uint8_t messagePos = 0; // Track current message read position
    uint8_t messageBuffer[259]; // Max message length for Moppy messages is 259
    uint8_t pongBytes[8] = {START_BYTE, 0x00, 0x00, 0x04, 0x81, DEVICE_ADDRESS, MIN_SUB_ADDRESS, MAX_SUB_ADDRESS};
    void sendPong();


};
  extern MoppySerial TheMoppySerial;
#endif /* SRC_MOPPYNETWORKS_MOPPYSERIAL_H_ */

It seems that I have trouble understanding you. Do you have a bit more details about that constructor thing and where args should go ?

The IDE throws me these errors at compilation :

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\MoppySerial.cpp:14:13: error: no matching function for call to 'MoppySerial::MoppySerial()'

 MoppySerial TheMoppyMoppySerial;

             ^

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\MoppySerial.cpp:14:13: note: candidates are:

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\MoppySerial.cpp:8:1: note: MoppySerial::MoppySerial(handleSystemMessage, handleDeviceMessage)

 MoppySerial::MoppySerial(handleSystemMessage sys, handleDeviceMessage dev) {

 ^

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\MoppySerial.cpp:8:1: note:   candidate expects 2 arguments, 0 provided

In file included from C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\MoppySerial.cpp:1:0:

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\../MoppyNetworks/MoppySerial.h:19:7: note: constexpr MoppySerial::MoppySerial(const MoppySerial&)

 class MoppySerial {

       ^

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\../MoppyNetworks/MoppySerial.h:19:7: note:   candidate expects 1 argument, 0 provided

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\../MoppyNetworks/MoppySerial.h:19:7: note: constexpr MoppySerial::MoppySerial(MoppySerial&&)

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks\../MoppyNetworks/MoppySerial.h:19:7: note:   candidate expects 1 argument, 0 provided

Lothean:
It seems that I have trouble understanding you. Do you have a bit more details about that constructor thing and where args should go ?

Something like:

class MoppySerial {
  public:    MoppySerial();
    void begin(handleSystemMessage sys, handleDeviceMessage dev, long baud = MOPPY_BAUD_RATE);
MoppySerial::MoppySerial() {
  messageBuffer[0] = START_BYTE;
}

MoppySerial TheMoppySerial;

void receiveEventHandler(int howMany) {
  TheMoppySerial.receiveEvent(howMany);
}

void MoppySerial::begin(handleSystemMessage sys, handleDeviceMessage dev, long baud) {
  systemHandler = sys;
  deviceHandler = dev;
  Wire.begin(DEVICE_ADDRESS);
  Wire.onReceive(receiveEventHandler);
  }

It does not really accept that.
I don't really know my way around since this is new to me (this whole constructor thing); but it seems like something is wrong in the void declaration?

Of course, I also tried different things in the declarations (like removing everything before or after the spaces and just leaving "sys" or "handleSystemMessage" but it didn't help

Since you are now treating TheMoppySerial as a singleton instance, the calling sketch shouldn't attempt to construct another instance of MoppySerial. Just use TheMoppySerial and call begin() to configure it.

Shouldn't they be args in the begin function ?

Of course, as defined in the MoppySerial class.

Okay, first, I'd like to say congratulation for you "God Member" status ! :stuck_out_tongue:
Secondly, I'd like to bother you one more time by asking (sorry!) : how do I declare properly these lines (the one I put arrow before them) ? :slight_smile:

void MoppySerial::receiveEvent(int howMany) {
    uint8_t payload[8]; // TO REDECLARE SOMEWHERE ELSE BEFORE FIRST UPLOADING
    uint8_t message; 
      if (Wire.read() == 1) { // if 1, it's a system event
        message = Wire.read();
        byte i = 0;
        while (Wire.available() > 0)
        {
          payload[i] = Wire.read();
          i++;
        }
        --> systemHandler(message, &payload[0]);
      }
}
void MoppySerial::begin(handleSystemMessage sys, handleDeviceMessage dev) {
  --> systemHandler = sys;
  --> deviceHandler = dev;
  Wire.begin(DEVICE_ADDRESS);
  Wire.onReceive(receiveEventHandler);
  }

Since I'm asked to do this here :

C:\Users\Lothean\AppData\Local\Temp\ccB977Si.ltrans0.ltrans.o: In function `receiveEvent':

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks/MoppySerial.cpp:29: undefined reference to `TheMoppySerial'

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks/MoppySerial.cpp:29: undefined reference to `TheMoppySerial'

C:\Users\Lothean\AppData\Local\Temp\ccB977Si.ltrans0.ltrans.o: In function `begin':

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks/MoppySerial.cpp:33: undefined reference to `TheMoppySerial'

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks/MoppySerial.cpp:33: undefined reference to `TheMoppySerial'

C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks/MoppySerial.cpp:34: undefined reference to `TheMoppySerial'

C:\Users\Lothean\AppData\Local\Temp\ccB977Si.ltrans0.ltrans.o:C:\Users\Lothean\AppData\Local\Temp\arduino_build_25184\sketch\src\MoppyNetworks/MoppySerial.cpp:34: more undefined references to `TheMoppySerial' follow

collect2.exe: error: ld returned 1 exit status

putting TheMoppySerial. before does not work!

Please post all your code.

Oh, yes, sorry!
MoppySerial.cpp :

#include "../MoppyNetworks/MoppySerial.h"
#include <Wire.h>
/*
 * Serial communications implementation for Arduino.  Handler
 * functions are called to consume system and device messages received from
 * the network.
 */
MoppySerial::MoppySerial() {
  messageBuffer[0] = START_BYTE;
}

MoppySerial TheMoppyMoppySerial;

void receiveEventHandler(int howMany) {
  TheMoppySerial.receiveEvent(howMany);
}

void MoppySerial::receiveEvent(int howMany) {
    uint8_t payload[8]; // TO REDECLARE SOMEWHERE ELSE BEFORE FIRST UPLOADING
    uint8_t message; 
      if (Wire.read() == 1) { // if 1, it's a system event
        message = Wire.read();
        byte i = 0;
        while (Wire.available() > 0)
        {
          payload[i] = Wire.read();
          i++;
        }
        systemHandler(message, &payload[0]);
      }
}
void MoppySerial::begin(handleSystemMessage sys, handleDeviceMessage dev) {
  systemHandler = sys;
  deviceHandler = dev;
  Wire.begin(DEVICE_ADDRESS);
  Wire.onReceive(receiveEventHandler);
  }

MoppySerial.h :

/*
 * MoppySerial.h
 *
 */

#ifndef SRC_MOPPYNETWORKS_MOPPYSERIAL_H_
#define SRC_MOPPYNETWORKS_MOPPYSERIAL_H_

#include <stdint.h>
#include "Arduino.h"
#include "../../MoppyConfig.h"
#include "MoppyNetwork.h"

#define MOPPY_BAUD_RATE 57600

typedef void (*handleSystemMessage)(uint8_t, uint8_t[]);
typedef void (*handleDeviceMessage)(uint8_t, uint8_t, uint8_t[]);

class MoppySerial {
  public:
    
    MoppySerial();
    void begin(handleSystemMessage sys, handleDeviceMessage dev);
	void receiveEvent(int howMany);
  private:
    handleSystemMessage systemHandler;
    handleDeviceMessage deviceHandler;
    uint8_t messagePos = 0; // Track current message read position
    uint8_t messageBuffer[259]; // Max message length for Moppy messages is 259
    uint8_t pongBytes[8] = {START_BYTE, 0x00, 0x00, 0x04, 0x81, DEVICE_ADDRESS, MIN_SUB_ADDRESS, MAX_SUB_ADDRESS};
    void sendPong();


};
  extern MoppySerial TheMoppySerial;
#endif /* SRC_MOPPYNETWORKS_MOPPYSERIAL_H_ */

MoppyCore.cpp :

( ....)
// Standard Arduino HardwareSerial implementation
#include "src/MoppyNetworks/MoppySerial.h"
//MoppySerial network = TheMoppySerial(instrument.systemMessage, instrument.deviceMessage);

//// UDP Implementation using some sort of network stack?  (Not implemented yet)
// #include "src/MoppyNetworks/MoppyUDP.h"
// MoppyUDP network = MoppyUDP(instrument.systemMessage, instrument.deviceMessage);


//The setup function is called once at startup of the sketch
void setup()
{
	// Call setup() on the instrument to allow to to prepare for action
    instrument.setup();

    // Tell the network to start receiving messages
    TheMoppySerial.begin(instrument.systemMessage, instrument.deviceMessage);
}

// The loop function is called in an endless loop
void loop()
{
	// Endlessly read messages on the network.  The network implementation
	// will call the system or device handlers on the intrument whenever a message is received.
}

You've over-moppyed it:

MoppySerial TheMoppyMoppySerial;

Argh! Gosh! What a stupid mistake of mine!
Thanks a lot for your help, arduarn, solved! :slight_smile: