Manchester decoding should work on bitrow not entire bitbuffer

[obones]

As discussed in #1726, manchester decoding functions require a bitbuffer_t to place the result of their decoding, but only ever use the first row in that buffer.

This pull request addresses this by having those method require a uint8_t array instead, which allows for far lower stack usage throughout the program.

Note that it introduces the NUM_BYTES macro already suggested in #2378 as it is used in all decoders that now require a uint8_t array whose size should be able contain the expected number of bits to be decoded. Once on of the two PR is merged, I'll gladly rebase the other one to avoid any conflicts and give a cleaner history path.

[zuckschwerdt]

Thanks! Much needed.

I still think that numbers of bits and bytes are an integral core of decoder operations and not something to be abstracted away (as e.g. NUM_BYTES(EXPECTED_NUM_BITS).
There will be cases where some extra bits are wanted or need to be cut off. I don't see us declaring "this is a 72-bit-message" protocol somewhere and then abstract the code to be oblivious of actual lengths and positions with universal success.

Or easier to discuss: same with CRC's where I don't like to see crc8(b, n, THE_POLY, THE_INIT). The indirection does not help, writting crc8(b, n, 0x31, 0) is one line less code (the define) and I can actually grasp that line instantly (and confirm that the poly is sane and the init elegant).

I realize this is far on the side of personal preference, but maintaining that may decoders needs to be dumb simple. E.g. I often grep or regex-replace int the sources and defines don't work with that approach.

[obones]

Well, I totally understand your point with regards to the CRC, but when it comes to number of bytes required to store a number of bits, I prefer to be on the (very) conservative side.
With hard coded numbers, it's all too easy to increase the number of bits given as an argument to bitbuffer_manchester_decode while forgetting to increase the destination uint8_t array.

And I find it clearer to have the NUM_BYTES construct instead of a somewhat cryptic operation that is all too easy to get wrong (see #2378)

Also, some decoders already have constants for the expected number of bits (or bytes) they are working with which I do not find inconvenient to read. If I really want the actual value, hovering the mouse on that symbol makes my IDE give me the actual value. See ced7000, danfoss, schraeder for example.

I'm not one of those "no magic numbers" fanatic because I loathe #define ONE 1 but I'm always in favor of code that helps me avoid out of bounds memory writes.

In the end, it's your call, just let me know.

[zuckschwerdt]

Good and valid arguments, I do see that too -- that's why it's so hard to decide. I really don't want number defines and macros, but they are reasonable and could be nice maybe.

[obones]

How about I modify the other PR (#2378) to have the changes limited to devices files, leaving alone bitbuffer and decoder_util?
The NUM_BYTES macro could be defined in decoder.h for instance and so only decoders would use it to safeguard themselves from forgetting to update the size of the manchester decoding buffer when adjusting the number of bits it expects.

Would that be acceptable?