Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datatype TEXT / Switch to CBOR #187

Open
hakha4 opened this issue Nov 23, 2023 · 3 comments
Open

Datatype TEXT / Switch to CBOR #187

hakha4 opened this issue Nov 23, 2023 · 3 comments

Comments

@hakha4
Copy link

hakha4 commented Nov 23, 2023

Hi!
Stumbled over FRDS and It looks very nice,Got a basic setup to work without issues. I'm working on a ESPnow node with mcp23017 to manage multiple relays and it seems to work fine but I miss a datatype TEXT though, I want to send relaystatus as a binary string like '001100110010011' from node for further manipulation. Is it hard to add this datatype?
Best Regards Håkan

@timmbogner
Copy link
Owner

This issue was discussed further in #188. I've decided to switch to CBOR to encode the data portion of the DataReading object. My current method of moving all data as floats is not ideal, and I've never liked it. I'm leaving this issue open until I make those changes.

If anyone has questions or suggestions regarding this topic, this is the place for it!

@timmbogner timmbogner changed the title Datatype TEXT Datatype TEXT / Switch to CBOR Jan 27, 2024
@bvwelch
Copy link

bvwelch commented May 13, 2024

I've used cbor on a project or two. It wasn't so bad. I used a library named tinycbor, and I also wrote a small library straight from the rfc. lemme know if i can help.

@timmbogner
Copy link
Owner

timmbogner commented May 17, 2024

Thanks @bvwelch, that's great to hear! This seems like a good time to get an overview of my thoughts on this down on paper. I'm going to be pretty thorough in case anyone else wants to join the discussion. It also helps me work through some of these issues in my head:

Since its conception, FDRS has used the 32-bit float type for every datapoint. This is flexible for casual use, and allows for smooth conversion to and from JSON. There are some serious downsides, however. Floats begin to lack precision at higher numbers and rounding errors cause unexpected results. We can't provide functionality for both floats and integers without adding extra data for context. My first plan was to borrow a couple bits from the 'type', but @shaffenmeister suggested encoding with CBOR and it seems way better (and standardized). Using CBOR is a way to provide a way for the user to manipulate all 32 bits of the 'data' portion of each DataReading (the primary data structure within FDRS) while still providing context as to what form of data is present.

Using CBOR to serialize the data, we could handle data as a raw uint32_t, a signed int, a float, or a boolean. Using floats will still be the "default" behavior for all existing functions in order to maintain backwards compatibility. Each of these types will result in a CBOR values of size 5 bytes or less. I also want there to a functionality for short strings, but I plan to add that after the general details are ironed out.

One area of complication for this will be when a controller receives some data. Currently the callback function for ESP-NOW reception is given the raw float, which is simple to use within the user's sketch. When working with raw CBOR, I'm still deciding how to hand it to the user. For backwards compatibility, it will be best to continue giving the user the DataReading with the data portion resolved to a float. Then, additionally, the user will be able to access the data in integer or boolean format, plus be given the intended type of the data. This felt a lot more complicated before I typed it out :)

I think loading different types of data into the system should turn out to be fairly simple. There can be several new versions of loadFDRS() that accept different data types as arguments. Each version will encode the data to CBOR in the format corresponding to the type it was given. (Revision:) There actually is another level I've thought of: loadFDRS() will probably need to take any positive integer that is entered as a float (i.e. '678.0') and encode it as a uint, and encode all negative integers as a signed int in CBOR.

The next thing that's challenged me is the process of converting between JSON-encoded DataReadings and CBOR-encoded ones. A JSON value can be a number, boolean, string, array, or another JSON object. CBOR ints, uints, and floats will all be expressed as JSON numbers, and booleans will be booleans in the new functionality. The complicated part is the conversion from JSON 'number' to a CBOR type. The logic will follow the revision above: any decimal number will be stored as a CBOR float, negative integers will be stored as signed int, and positive integers will be unsigned ints. Strings will eventually be straightforward to handle too, and probably delivered as a C string or char array.

The DataReading is currently comprised of a 16-bit identifier, an 8-bit type, and a 32-bit float to hold the data. That makes only 7 bytes total, which (in my mind) leaves us a free byte that we can add to this object. My proposal is to keep the first two elements (id and type) as-is, but change the data portion to uint8_t data[5]. The five bytes of the data portion will now contain the CBOR-encoded data. I think the correct way to re-organize things within code will be to rename the new DataReading CborDataReading. A "classic" DataReading structure will still exist, but it will have added bool, uint32, and int elements. The "classic" DataReading will be the one accessed by the user, while the CborDataReading will be used in buffers and transmissions. Since gateways never inspect the data they're repeating, they should be minimally impacted by these changes. Gateways expect packets from the radio to be subdivided into blocks of size sizeof(DataReading). A lot of lines will have to be changed to sizeof(CborDataReading).

Adding a byte to the data portion will, of course, cause all new devices to no longer be compatible with old ones. I don't take this lightly, but I feel that the extra capabilities are worth the hassle to the user. It might also be a good reason for me to start keeping version numbers and doing releases.

@aviateur17 I think we'll be able to send a timestamp (like to accompany data) as a uint32_t when this is all done, so that will be a big plus! TIMESTAMP_T

I think that wraps up my ideas, thanks for reading. If anyone has anything to add or critique, please do! I plan to use tinyCBOR, but I'm not sure if it could be easier (or lighter) to implement CBOR manually. I'll probably keep talking and thinking about this for a month or so before I actually start working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants