Refactor LedStateParser

[4c3y]

I smell a problematic design here since this function doesn't have any input. I tried to understand why and found that the processImage function of the LedStateParser changes its internal state. That isn't a good idea IMO. I don't have the space to explain it here but I'd highly recommend having a dedicated class representing the outcome of the an processImage and that processImage is a const member of the LedStateParser.

It is very uncommon for a parser to represent the parse outcome as internal state and there are many good reasons for that. I'm happy to explain one day in person. I think in very short, this is about separating data (led state, images) from actors/entities (e.g. a parser).

This function here, could then be a member of that outcome class iff its members would hold all relevant information. Otherwise consider passing such a parse outcome to it as an argument instead.

I mostly found a potential problem with the convex shape. I'm not sure what is_valid actually does, but is it actually interesting to receive led parser results when the convex shape is invalid? It sounds to me that parsing doesn't really make sense when the shape is invalid. Or does it?

The process function of the ImageProcessor could simply return both, the let-state and the Image message it currently returns (e.g. using a std::tuple or - probably better - a small struct holding both with proper names). More symmetric would probably be if the process simply returns both messages: the output image and the led state message.

Originally posted by @HannesSommer in #10 (comment)

[michaelpantic]

Before refactoring, check, and potentially extend your set of unit tests.

Ideally there are even multiple level of tests:

• Unit tests that just test individual functions with a few samples of valid/invalid data
• Integration tests that test a entire component with sample input (i.e. the whole LedStateParser)
• (not for now, but a good exercise): System tests, that may only be partially automatized, that e.g. test the whole node based on a rosbag in a script.