Could inherit from ECSS Message claas, could be a template. Find out the optimal solution Convertation: @toniker : There is an idea for a refactor, in which a CAN-TP Message is actually a `Message` from `ecss-services`. This approach has some pros-cons and we would like some input on which direction to follow. Pros * Easy appending and reading of different types of variables using `message.readUint16()` etc. * Error handling is already implemented. * Executing a TM/TC requires an ECSS Message either way, so why not already build one with the required data. Cons * The current `Message` implementation has a data size of 1024 bytes. This is far more than we could possibly need in a CAN Message and would be wasteful of memory. * We would need to use a single `static Message` and create an empty function each time we process a new CAN-TP Message for memory reduction. * We're using an ECSS Message in a place where not all of it's functionality is needed. We could possibly create a `TPMessage` class that essentially copies the functionality we want, but that's code duplication. (This approach would probably have to be included in ecss-services, in order to use the parameter append functions. However the class doesn't belong in `ecss-services`.) @sourland and I are open to the idea, however we would like some input from more knowledgeable people. ---------------------------------------------------------------------------------------------------------------------------------------------- @elefthca : I'm not really qualified to speak on OBC architecture so Ill leave this one to the masters. At least you managed to convince me that such a thing would be a good idea. On your code duplication point, perhaps you could template the `Message` class (don't know how much of a refactoring effort this would be) and initialize it once with the 1024 default size and once with your desired CAN message size. Also perhaps you could have a `ECSSMessage` class (the naming is up to you) that derives from your `Message` class so you could avoid carrying over functions that won't be called (though perhaps compilers usually optimize this?) Also, since you'd be essentially using a single instance for all your handled messages I guess this object's buffer would need to be reset/rewritten each time. I assume this is an easy and fast operation and you wouldn't need to do something like popping bytes one bye one right? To insert my personal biases a bit, I think the code is fine as-is and it wouldn't warrant the need for a fairly big refactoring. You do however need extensive tests and by skimming the code, there is quite a lot of unsafe pushing to and popping from buffers so I'd personally run through the code again trying to identify possible failure points Fyi, in comms we used raw buffers for storing ccsds packets. It allows a certain degree of freedom but we were also forced to re-invent a more dangerous and more square wheel so I dont want to downplay the importance of re-using tested and true code But again, this is just an outsider's opinion and I can't really "push" you in one direction or the other so better wait for the *masters* ----------------------------------------------------------------------------------------------------------- @sourland : 2 major concerns: * What happens when CAN-Bus traffic is high.. We can't have the `incomingMessages` map as a Message map since a Message is a VERY LARGE object. * Since everything is running in a pseudo-parallel style, what will happen if another task jumps in and append bytes between the message saving procedure? ----------------------------------------------------------------------------------------------------------- @NikosStrimpas86 : I'm not qualified to speak on OBC architecture either, and especially CAN stuff but since I was tagged I can write what I think is the most logical step for a refactor. It's not really the *modern C++* way but I think using a base class that implements some common behavior, like error handling, parsing, reading, writing etc. and then having some classes like `ECSSMessage` and `CANTPMessage` inheriting that (not using rtti of course) would make sense. As a bonus the base class could be templated, giving you compile time information and maybe helping with the excessive memory use problem that you described. This seems to me like a logical step, but I can tell that it's going to be a long one, so you have to choose between your time and the compactness of your code, as well as the elegance of your abstractions. I can't help but to agree with @elefthca that your buffer pushing/popping situation is concerning though, and "fine grained" testing should be done. Try to fit as much stuff at compile time when it comes to potential errors. This was what I had, to say. Just like you, I'll wait for what the seniors have to say. ----------------------------------------------------------------------------------------------------------- @kongr45gpen : string