draft MSPv2 support

[humanoid2050]

This has most of my internal changes to support MSPv2. Converted all messages to inherit from msp::msg::Message class. Turned ByteVector into an actual class. Added a new value type to support optional decoding of message fields. No changes to MSP, Client, and FlightController APIs.

[christianrauch]

There are quite a lot of changes. I won't be able to address all of them now. But thanks for contributing to begin with :-)

My comments so far by looking on the diff:

• Which feature of C++14 is required? Could we go on with C++11 instead?
• Why does the ByteVector need to have the additional methods?
• Style: (1) Could you format comments with a space after the double slash. I.e. // coment. (2) Could you remove some of the blank lines such that there are no more than a single blank line between methods/function/variables?
• Does Client need to store the firmware type (member firmware) or could this be done in the FlightController class? I would like to keep the Client class as generic as possible. I.e. the Client class doesn't know anything about message types, it just does the communication. The FlightController on the other hand is actually determining the firmware type and all required metadata.
• For some messages (e.g. RxMap), the firmware type is given via the constructor but it appears to me that it is never used within this message.
• Messages now have a print method, and we have the stream operator defined in msg_print.hpp. What is the advantage of the print method over the operator? Can we remove the print method from the messages and use the dedicated stream operators for printing messages?

I decided to but the message IDs (enum msp::ID) in its own header so that it can be available without loading all the message definitions. We could use a forward declaration of the enum class msp::ID in message.hpp and move the IDs into the header for the message definitions (msp_msg.hpp). This would gather all the message definition stuff that might change with firmware in a single header file.

I will test your changes later on the most recent betaflight release.

Edit: For future pull requests, I would recommend to use a dedicated feature branch (with a reasonable name) and keep your master branch in sync with the upstream project.

[humanoid2050]

To address your comments,

• I think c++14 was so that I had access to std::make_unique. They forgot to put that into c++11.
• ByteVector also has additional data: an integer tracking the offset of the unpacking. Whenever someone calls ByteVector::unpack(T& destination), the ByteVector automatically consumes bytes so the next unpack starts where the previous one left off. Then the user doesn't have to manually calculate the offsets, they just have to unpack to the appropriate destination in sequence.
• Sure thing on the style. I have no particular feelings about such things.
• I think you are right about Client not needing to know about the firmware variant
• I made passing a firmware argument default for all Message constructors. It's not used in a lot of them. Might be an argument in favor of separating the message implementations into firmware-specifc libraries.
• Having a virtual print method defined in msp::msg::Message meant that I could do a single method definition std::ostream& operator<<(std::ostream& s, const msp::Message& val) { return val.print(s); }; and any message with a virtual print override could be printed. If the message didn't have its own definition, the parent method would be called with no apparent side effects.
• I'm flexible about moving things around to better suit long term support.
• I actually do have a MSPv2_support branch. It just happens to have even more changes (mostly API oriented) so I made an intermediate merge back toward master. Probably would have been smarter if I used a staging branch...

[christianrauch]

• Does it mean that you need to create every message by providing the firmware version in the constructor, even though there might be no difference among the message definitions?
• I am somehow not happy with having the print function inside the message definition. I prefer dedicated streaming operators. If a user decides to use the streaming operator, he must make sure that it exists for the message or a compiler error is thrown otherwise. If you just use the print function and you do not get output on the screen, you do not know if it is because of the missing implementation or because you simply did not receive any messages of that type.
• If this PR does not introduce MSP2 support, could we remove all MSP2 references (e.g. the uint16 ID) and postpone this feature for a later MSP2 feature pull request?

[humanoid2050]

• As written, yes. I just did that for uniformity.
• Fair point about print. My thought is that we may also have to split the msg_print library into firmware-specific versions as well.
• Derp... I should have updated the Client library to have the MSPv2 code paths. Do you want to check out the MSPv2_support branch from my fork? That has everything, and I can make adjustments there before doing pulls into master.

[christianrauch]

How large is your MSP2 feature branch?
If it helps you, you could create a new MSP2 pull request that adds MSP2 support and everything that needs to be changed to enable MSP2 (e.g. including this PR).

It's just that it becomes difficult to review the PR if it adds too many changes.

[humanoid2050]

Its only 25ish files including examples, about like master is now. There is a lot of resemblance to the master code base, but its all been changed in subtle but pervasive ways.

[christianrauch]

You can give it a try. If it's too much, I will continue with this PR.

[humanoid2050]

ok so new PR against my full MSPv2 changes?

[christianrauch]

Yes. Can you create a new PR (and leave this open for now) that includes all the MSP2 support changes? I.e. the features that you originally wanted to merge, where I initially proposed to split these changes into multiple PRs. Let's try to do all of these changes in a single PR.

[christianrauch]

Superseded by #28.