Strange behaviour in class properties

Hi,

I am building some software to send and receive CAN messages. Therefore I have created 2 libraries myself and I am using the MCP_CAN library (GitHub - coryjfowler/MCP_CAN_lib: MCP_CAN Library). I will give a short explanation about my own written libraries.

I have a class called CAN_Message which keeps the basic function of a CAN message frame. Then I have a publisher and a subscriber class. The publisher has properties latency and callback. If i.e. the latency is 1000, the callback function should be called every 1000 ms in which you can program yourself what the publisher should do (probably to send a CAN message). The subscriber has as properties can_id and callback. The idea is that when a message with id can_id has been received, the callback function of the subscriber is called.

Then I have a class called CAN_Node_Handler. The node handler maintains a list of publishers and a list of subscribers. In the loop it should check for every publisher if it is time to call the publisher callback function. The node handler also should be notified about a new CAN message, so it can check for all of the subscribers if the callback function should be called.

The list of publishers is a simplified implementation of c++'s std::vector with just the stuff I need. As you might notify I use publisher pointers and subscriber pointers in the node handler. This is because the pass by reference didn’t seem to work properly. Everytime I got one publisher out of the list of publishers it seemed like it has been copied. I fixed it using pointers.

Now the problem. In the loop I first spin the publishers, so I check for each publisher if it is time to call the callback function and if it is, I call the callback function. Everything goes alright, messages are sent correctly with use of the MCP_CAN_lib. Then I try to receive a message. If there is a message (digitalRead(2) == false) I receive it. This also goes alright, I can receive messages with MCP_CAN_lib. But from the moment I have tried to receive a message with MCP_CAN_lib (the first time digitalRead(2) == false) the values of my publisher’s properties have changed (see the serial prints). I have no clue why this is. I suspected maybe I used too much space for the variables, so I reduced the size of the publisher and subscriber lists, but this didn’t help.

If you don’t have an MCP can module, you can still reproduce this. You just have to connect the ground to D2.

Can anyone help me with this? I have attached all of the code I have used.

can_container.h (783 Bytes)

can_message.h (2.93 KB)

can_node_handler.h (2.18 KB)

can_publisher.h (1.04 KB)

can_subscriber.h (807 Bytes)

rtb_can.h (419 Bytes)

rtb_can_database.h (873 Bytes)

rtb_can_dfs.h (131 Bytes)

rtb_lib_test.ino (2 KB)

rtb_functions.h (1.35 KB)

well what you posted does not compile and it’s tedious to look at so many files…

I had to fix some <> to “” for the includes and get rid of those includes which you did not provide (but they seem useless)

//#include "remote_request_frame.h"
//#include "odrive_heartbeat_message.h"
//#include "get_vbus_voltage_message.h"

this now compiles but there are warnings:

[color=orange]In file included from sketch/can_node_handler.h:7:0,
                 from sketch/rtb_can.h:8,
                 from ~/rtb_lib_test/rtb_lib_test.ino:1:
sketch/can_publisher.h: In member function 'CAN_Publisher& CAN_Publisher::operator=(const CAN_Publisher&)':
sketch/can_publisher.h:31:2: warning: [color=red]no return statement in function returning non-void[/color] [-Wreturn-type]
  };
  ^

In file included from sketch/can_node_handler.h:8:0,
                 from sketch/rtb_can.h:8,
                 from ~/rtb_lib_test/rtb_lib_test.ino:1:
sketch/can_subscriber.h: In lambda function:
sketch/can_subscriber.h:16:47: warning: unused parameter 'message' [-Wunused-parameter]
     : CAN_Subscriber(0, [](const CAN_Message &message){}) { };
                                               ^~~~~~~
sketch/can_subscriber.h: In member function 'CAN_Subscriber& CAN_Subscriber::operator=(const CAN_Subscriber&)':
sketch/can_subscriber.h:25:2: warning: [color=red]no return statement in function returning non-void[/color] [-Wreturn-type]
  };
  ^

In file included from sketch/rtb_can.h:8:0,
                 from ~/rtb_lib_test/rtb_lib_test.ino:1:
sketch/can_node_handler.h: In member function 'bool CAN_NodeHandler::add_publisher(const CAN_Publisher&)':
sketch/can_node_handler.h:28:38: warning: invalid conversion from 'const CAN_Publisher*' to 'CAN_Publisher*' [-fpermissive]
         return _publishers.push_back(&publisher);
                                      ^~~~~~~~~~

In file included from sketch/can_node_handler.h:9:0,
                 from sketch/rtb_can.h:8,
                 from ~/rtb_lib_test/rtb_lib_test.ino:1:
sketch/can_container.h:29:10: note:   initializing argument 1 of 'bool CAN_Container<T>::push_back(const T&) [with T = CAN_Publisher*]'
     bool push_back(const T &value) {[/color]

I would suggest you start there and get rid of all warnings - especially the ones about “no return statement in function returning non-void”.

Once you have zero warning left, double check the amount of memory at compile time and run time and if not approaching limits, then it will be time to start digging more into where the bug might occur.

One thing to explore is your “Copy assignment operator”.

	// Copy assignment operator
	CAN_Publisher& operator=(const CAN_Publisher &other) {
		// _latency = other._latency;
		// _callback = other._callback;
		// _counter = other._counter;
		*this = other;
	};

You probably don’t want to overwrite this, you should perform a copy of your instance variables ( _latency, _callback and _counter) as you commented out and return *this;.

Thank you for your reply!

it’s tedious to look at so many files…

In contrast to stackoverflow everybody here always asks to post ALL the code, so that is why I did it. Also I didn’t get a compiler error or warning (now I know I can change compiler settings in preferences) so I really didn’t understand what was going on, could be anywhere in the code.

I had to fix some <> to “” for the includes and get rid of those includes which you did not provide (but they seem useless)

Sorry about this, should have commented them out. But indeed, I didn’t use them.

I would suggest you start there and get rid of all warnings

This is a really good comment. Thank you for this. I changed the compiler settings to show all warnings in stead of showing nothing and this is really valuable to check your own mistakes.
I fixed all warnings and right now I have 1 warning left. In the empty constructor of Publisher I pass a function with argument, but I don’t do anything with that argument because I want this function to be empty.

CAN_Publisher()
    : CAN_Publisher(1000, [](){}) {};

The compiler gives me a warning:

can_subscriber.h:16:47: warning: unused parameter ‘message’ [-Wunused-parameter]

: CAN_Subscriber(0, (const CAN_Message &message){}) { };

but I don’t think I can fix this warning, only suppress or ignore it.

You probably don’t want to overwrite this, you should perform a copy of your instance variables ( _latency, _callback and _counter) as you commented out and return *this;

Yes indeed, my mistake. I find it weird that the compiler didn’t give an error message. Because I think this operator is calling itself recursively.

But after all, I have fixed all the warnings except this one I just described and I don’t get the weird behavior anymore. From now on I will let the compiler show all the warnings. Thanks for your help!

Fair point on the full code - usually we ask for minimal code demonstrating the issue. There was lots to read here. But fine.

Glad it seems fixed !!!

I think the error was really in the Copy assignment operator.

If you want to remove the last warning, there is a way to tell the compiler to ignore the fact that one parameter is not used. This is accomplished by putting [color=blue]__attribute__ ((unused))[/color] just before the parameter. For example, instead ofvoid aFunction(int x) { }you would do void aFunction(__attribute__((unused)) int x) { }
That tells the compiler you’ve done this knowingly and you won’t get the warning.