BPIO2 binary mode

Installed flatcc on Ubuntu with the monster example project. Managed to compile that. Trying to find out how to integrate it now. Mention of use on Arduino, so maybe looking at that might be useful.

Make some progress, perhaps?

  • Installed flatc and flatcc
  • Studied this Arduino library which is code only (no linked libraries).
  • Copied flatcc/include/flatcc folder to Bus Pirate project /src/flatcc
  • Copied flatcc/src/runtime files to Bus Pirate /src/ folder
  • Installed and compiled mymonster example. Copied the files from mymonster/generated to Bus Pirate /src/ folder
  • Got really sidetracked because the C example here is out of date/incomplete
  • Noticed the /mymonster/src/monster.c demo file. Source works, copied into a new flat command
  • It appears to work?

I have not:

  • Tried to de-serialize it
  • Customized any flatbuffer fields

Current work is on a new branch flatcc in git, the example is run from /commands/global/flat.c

Has anyone worked with flatbuffer before? It would be super nice to have a simpler example to build on for our needs. For example one I2C read write transaction with X bytes write and Y bytes read.

Deserialized successfully.

table I2CTrans{
  start:bool; // Start condition.
  data:[ubyte]; // Data to read or write
  readbytes:uint32; // Number of bytes to read.
  stop:bool; // Stop condition.
  error:bool; // Error flag. (not needed, check length of error_message?)
  error_message:string; // Error message if any.
}

I’m sure it will become more complex and efficient over time, but maybe this is a good starting place for an I2C test?

I suggest separate tables: one for the command and one for the response. E.g.,

table I2CRWRequest {
  start:bool; // Start condition.
  addr:ubyte; // Device address (Bus Pirate automatically will set read/write bit)
  data:[ubyte]; // Data to write
  readbytes:uint32; // Number of bytes to read.
  stop:bool; // Stop condition.
}

table I2CRWResponse {
  ack:bool; // Device acknowledged
  data:[ubyte]; // Data read from device
  error_message:string; // Error message if any.
}
1 Like

You could test for the presence of any error_message as the indicator for if there was any error. In flatbuffers, all fields are optional.

1 Like

Thank you so much! That got me on the right path. A demo serializing 10 bytes of write data, then reading back from the serialized buffer.

table I2CRWRequest {
  i2cstart:bool=true; // Start condition.
  i2caddr:ubyte; // Device address (Bus Pirate automatically will set read/write bit)
  i2cdata:[ubyte]; // Data to write
  i2creadbytes:uint32; // Number of bytes to read.
  i2cstop:bool=true; // Stop condition.
}

Added a default value for start and stop.

I suppose if the address is not populated, we can determine that and then skip sending the address? That would offer the flexibility to pack your own address into the i2cdata, which is what I usually prefer. Then the same packet can be used to just manipulate start, stop, or read/write another batch of data without start/stop. Is that in line with what you need @robjwells?

It seems like most modes might be able to share a common request/response packet.

Off the top of my header, other tables:

  • Request version, installed modes (ASCII list)
  • Request mode change
  • Configure mode (different for each?, or a general packet with a protocol struct stuck in it?)
  • PSU/pull-up/freq/PWM/voltage report/LED color, etc accessory requests
  • Maybe a facility to list/read/write files from the internal storage

Now that I have a handle on it, it seems super promising.

I have not had luck compiling flatc (non-c version), I’ll need to get that going so I can make a python test framework.

Any help with the higher level organization would be welcome.

Do we send different tables and test for type?

Do we have a master table with a version header, packet type enum, and then “attach” stuff like the I2C table above?

I see we can test if things are present, but it would be more efficient on device side to know what’s coming in and how to route it.

We can define a single Packet table that contains any one of various tables, via union, e.g.:

table I2CRWRequest {
  i2cstart:bool=true; // Start condition.
  i2caddr:ubyte; // Device address (Bus Pirate automatically will set read/write bit)
  i2cdata:[ubyte]; // Data to write
  i2creadbytes:uint32; // Number of bytes to read.
  i2cstop:bool=true; // Stop condition.
}
table I2CResponse {
  ack:bool; // Device acknowledged
  data:[ubyte]; // Data read from device
  error_message:string; // Error message if any.
}

union PacketContents { I2CRWRequest, I2CResponse }

table Packet {
  version:uint32;
  contents:PacketContents;
}

Search for contents_as in the generated .h file for convenient helper methods. :slight_smile:

2 Likes

Yes I think so. I had a (quick) look through the flatbuffer documentation today, hopefully tomorrow I’ll be able to pull together a dummy implementation/PoC.

1 Like

@bz2 recommended it.

And the documentation I excerpted mentions it also:

… a message wrapper with a union of tables holding buffer for a specific message type …

This helps reduce the overall size of the table.

Also, consider defining two top-level tables. One for communication TO the buspirate, and one for communication FROM the buspirate.

You can always combine them later, but testing may show that it reduces the buffer size needed to build one direction’s communications (experimentation needed).

2 Likes

namespace BPIO2;

table I2CRWRequest {
  i2cstart:bool=true; // Start condition.
  i2caddr:ubyte; // Device address (Bus Pirate automatically will set read/write bit)
  i2cdata:[ubyte]; // Data to write
  i2creadbytes:uint32; // Number of bytes to read.
  i2cstop:bool=true; // Stop condition.
}
table I2CResponse {
  ack:bool; // Device acknowledged
  data:[ubyte]; // Data read from device
  error_message:string; // Error message if any.
}

enum PacketType:uint8 {
  I2CRWRequest = 0, // I2C Read/Write Request
  I2CResponse = 1, // I2C Response
}

union PacketContents { I2CRWRequest, I2CResponse }

table Packet {
  version_major:uint8=0;
  version_minor:uint8=1;
  type:PacketType; // Type of packet
  contents:PacketContents;
}

root_type Packet;

Not sure this is the right thing to do, but I explicitly added a PacketType enum. I did manage to get the type from the flat buffer, however it seems like that requires testing each case against all possible packet types.

I would much rather build the Bus Pirate interface as an array of function pointers, and pass the packet to a function in the array based on the PacketType enum (shared between flatbuffer and the firmware).

Please let me know if there is a preferred way to do this.

Next I’ll try to get this working with a python script.

I think defining an explicit PacketType is not needed, as that enum is built in when using the union PacketContents. Excerpt from the auto-generated .h file:

enum PacketContents : uint8_t {
  PacketContents_NONE = 0,
  PacketContents_I2CRWRequest = 1,
  PacketContents_I2CResponse = 2,
  PacketContents_MIN = PacketContents_NONE,
  PacketContents_MAX = PacketContents_I2CResponse
};

// ...
struct Packet FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
  // ...
  PacketContents contents_type() const

Edited to add: I think this will still work for passing as an array index based on contents_type(), though we may want to add some sanity checks via macros or other verification.

1 Like
C:\flatpy>python flatpy.py
FlatBuffers packet created successfully! Buffer size: 64 bytes
Buffer contents (first 20 bytes): 1400000000000e00080000000000000000000400

Serial data format:
Length header (2 bytes): 4000 (value: 64)
FlatBuffer data (64 bytes): 1400000000000e000800000000000000000004000e000000100000000c00100000000f00040008000c0000000c0000000a000000000000500300000001020300
Complete packet (66 bytes): 40001400000000000e000800000000000000000004000e000000100000000c00100000000f00040008000c0000000c0000000a000000000000500300000001020300
Opened serial port COM35 at 115200 baud
Sent 2 bytes (length header): 4000
Sent 64 bytes (flatbuffer data): 1400000000000e000800000000000000000004000e000000100000000c00100000000f00040008000c0000000c0000000a000000000000500300000001020300
Total bytes sent: 66

A python script using the bpio2 flat buffer. Sends the I2C request to the Bus Pirate.

I see that framing is going to be a whole thing, but for the moment I’m sending packet size as a two byte prefix.

A hacked together decoder in binmode test framework.

I messed with the framing and see the need to use the validator functions.

Thank you! Mine .h is much more obfuscated than that, I couldn’t find an enum.

#define BPIO2_PacketContents_NONE ((BPIO2_PacketContents_union_type_t)UINT8_C(0))
#define BPIO2_PacketContents_I2CRWRequest ((BPIO2_PacketContents_union_type_t)UINT8_C(1))
#define BPIO2_PacketContents_I2CResponse ((BPIO2_PacketContents_union_type_t)UINT8_C(2))

I do have this. I do not have PacketContents_MIN at all though.

Next steps?

  • A configuration table that selects mode and relevant settings. (and generic response)
  • A configuration table to set power, pull-ups
  • Implement I2C mode and data

This would be the bare minimum to do a live test with an I2C EEPROM. I’ll probably have to stumble through how best to represent all that. I’ll start with henry’s suggestion to have separate request and response packets.

//new tables to query device status, select and configure a mode, enable power, etc.
table StatusResponse {
  //HW version
  //FW version
  modes:[Mode]; // List of modes available on the device.
}

table Mode{
  id:uint8; // Unique ID for the mode.
  name:string; // Name of the mode.
}

//use one or the other
table StatusRequest{
  id:uint8; // ID of the mode to query.
  name:string; // Mode enter, none for mode query.
  //mode spefific configuation union
}

Prototype of a Status interaction:

  • Host sends a blank/empty statusRequest table
  • BP replies with Status response with vector of Mode (id and name)
  • Host sends StatusRequest table with id and or name populated
  • BP changes mode, sends another status response with the current mode (not yet included)
ns_bpio(StatusResponse_start(B));
ns_bpio(StatusResponse_modes_start(B));
for (uint8_t i = 0; i < count_of(modes); i++) {
    flatbuffers_string_ref_t name = flatbuffers_string_create_str(B, modes[i].protocol_name);
    ns_bpio(StatusResponse_modes_push_create(B, i, name));
}            
ns_bpio(StatusResponse_modes_end(B));
BPIO2_StatusResponse_ref_t status_response = ns_bpio(StatusResponse_end(B));
    // add to packet wrapper
ns_bpio(Packet_start_as_root(B));
BPIO2_Packet_contents_StatusResponse_add(B, status_response);
ns_bpio(Packet_end_as_root(B));            
// send the packet
uint8_t* buf;
size_t len = flatcc_builder_get_buffer_size(B);
buf = flatcc_builder_finalize_buffer(B, &len);

Here is how I package everything up. I don’t feel like I have the optimal pattern figured out yet.

    uint8_t packet_type = ns_bpio(Packet_contents_type(packet));
    if(packet_type==ns_bpio(PacketContents_StatusResponse)){
        //get the array of modes from the status response
        ns_bpio(StatusResponse_table_t) status_response = (ns_bpio(StatusResponse_table_t)) ns_bpio(Packet_contents(packet));
        // Make sure the status response is accessible.
        test_assert(status_response != 0);
        ns_bpio(Mode_vec_t) modes = ns_bpio(StatusResponse_modes(status_response));
        // Make sure the modes vector is accessible.
        test_assert(modes != 0);
        // Iterate through the modes vector and print each mode.
        for (size_t i = 0; i < ns_bpio(Mode_vec_len(modes)); i++) {
            ns_bpio(Mode_table_t) mode = BPIO2_Mode_vec_at(modes, i);
            uint8_t id = ns_bpio(Mode_id(mode));
            const char* name = ns_bpio(Mode_name(mode));
            printf("Mode %zu: id=%d, name=%s\r\n", i, id, name ? name : "NULL");
        }
    }

Decoding the packet. I guess this is ok, and would be less verbose by verifying the table first?

I figured out how to get the packet type and removed my own ENUM from the flatbuffer.

1 Like

Do we need the high level (root) packet wrapper, or is there also a way to tell what kind of table is received? I searched around and the answer seemed to be no, but mostly in relation to streaming packets due to the reverse order of flat buffer data.

I think we need to always receive the same kind of table (table Packet from BBIO2 binary mode - #50 by ian , or direction specific root packet type per @henrygrab BBIO2 binary mode - #49 by henrygab ), with framing and/or checksum as desired.

My experience is the only way to tell what kind of table is received is to try to verify it (BUT see " Access of untrusted buffers" at C++ - FlatBuffers Docs ). Even then, a table might be “successfully” verified but actually is a different type. If you let your root-level table contain a union of other tables, you can rely on that for determining sub-table type.

1 Like

Thank you so much, that’s exactly what I needed to understand.

1 Like

Question on

If you later deprecate (and then remove) one of the types of supported packet types, what changes in the resulting enumeration?

If a decision is made to rely on this, could it later causes things to break horribly? e.g., if one version is released that accidentally defines its structure as 256bytes, how does one remove the support for that packet type? (simply removing it from the list of enum will break because any later enum values will change; redefining the structure size to be a single byte could break marshalling between versions; etc.)

In short, how to test that functionality (message / packet types) can be safely deprecated / removed?

I’m wondering if one rule is that a structure can never be made smaller?

Put another way: What constraints exist when using Flatbuffers, to ensure forward and backward compatibility?


A concrete example for discussions

namespace CompatibilityTest;
table LargeRequest {
  value:bool;
  data_001:[uint32];
  data_002:[uint32];
  data_003:[uint32];
  // ... 
  // plus a few more to make it large
  // ...
  data_080:[uint32];
}
table Request1 {
  value:bool=true;
  data_to_send:[ubyte];
}
table Response1 {
  result:uint32;
  data_response:[ubyte];
  error_message:string;
}
table Request2 {
  value:bool=true;
  data_to_send:[ubyte];
}
table Response2  {
  result:uint32;
  data_response:[ubyte];
  error_message:string;
}
table SemVer {
   ...
}
union PacketContents {
  LargeRequest,
  Request1,
  Response1,
  Request2,
  Response2
}

table Packet {
  version:semver;
  contents:PacketContents;
}

root_type Packet;

Steps and questions:

  1. Add Request3 and Response3
  2. Remove LargeRequest (maybe it was dangerous)
    • what’s the right way to safely do so?
  3. Remove field error_message from Response1.
    • what’s the right way to safely do so?

1 Like

Agreed, your comments are on point. Per Evolution - FlatBuffers Docs :

You MUST not remove a field from the schema, even if you don’t use it anymore. You simply stop writing them to the buffer.
[It’s] encouraged to mark the field deprecated by adding the deprecated attribute. This will skip the generation of accessors and setters in the code, to enforce the field not to be used any more.

It is unclear if union members can be marked as deprecated.

Step #2 “Remove LargeRequest” : Unknown: can we mark union members as deprecated?

Step #3 “Remove field error_message from Response1”: mark like so:

table Response1 {
  result:uint32;
  data_response:[ubyte];
  error_message:string (deprecated);
}
2 Likes

Outstanding evolution document! Wish that type of documentation was more common.

I want to call out two caveats that seems likely to me at this early phase of my learning flatbuffers (please correct me here!), and note two well-documented best practices.

Caveats?


Critical: When first defining a field (e.g., in a table), ensure default value "makes sense"

why

If the field is ever deprecated, senders using the evolved schema will not include the field, so a reader using the older schema will get the default value.

Similarly, if a field is being added, senders using the older schema will not include the field. As a result, an reader using the evolved schema will only get the default value.

Mitigation

The only mitigation seems to be to require that all consumers will always check for a field’s presence, prior to reading the value.

Unfortunately, I don’t think there’s a CodeQL query for that (yet), which makes enforcement substantially impossible.


Don't manually re-encode a new flatbuffer object from another flatbuffer object's fields manually

Why

Consider Alice using V2, and Bob using V1. Alice encodes three fields with non-default values. Data is serialized to file. Bob reads the file, and accesses the fields … but only two b/c Bob is using V1 of the schema. (Maybe Bob scales the values, or rounds to some precision, or …). Bob then manually creates a new flatbuffer object, and serializes that to that same file.

When Alice deserializes that file (using V2 schema), the default value of the third field is provided to Alice. This is a form of data loss … Alice’s prior non-default values are just … lost.

Mitigation

Don’t use flatbuffers in a data pipeline type scenario.

  • Embed SEMVER for each table, and treat table instances as read-only and non-exportable if they are from a newer (but compatible) version. Again … the problem here is compliance testing … thus essentially not something to be relied upon.

  • Explicitly disallow adding or removing fields to tables. Require new table types to be defined for those situations?

  • The documentation page states something, which might prevent the field’s disappearance … but would need to confirm not only current behavior, but if this is contractually guaranteed. Specifically, if building a new table from an existing table is schema-agnostic (copying both known and unknown field IDs … but not sure how if it doesn’t know the size for unknown field IDs):

It is possible to build new buffers using complex objects from existing buffers as source.


:tada: Good news! The flatc compiler has the option --conform OLDSCHEMA which errors out on some of these problematic evolutions! Thus, it should be easy to catch at least those problematic evolutions via CI checks (e.g., github action) :tada:

2 Likes
.fbs work in progress
namespace bpio;

//new tables to query device status, select and configure a mode, enable power, etc.
table StatusResponse {
  hardware_version_major:uint8; //HW version
  hardware_version_minor:uint8; //HW revision
  firmware_version_major:uint8;//FW version
  firmware_version_minor:uint8; //FW revision
  modes:[Mode]; // List of modes available on the device.
}

table Mode{
  id:uint8; // Unique ID for the mode.
  name:string; // Name of the mode.
}

table I2CConfig {
  speed:uint32; // Speed in Hz.
}

union ModeConfiguration { I2CConfig,}

//use one or the other
table StatusRequest{
  id:uint8; // ID of the mode to query.
  name:string; // Mode enter, none for mode query.
  configuration:ModeConfiguration; //mode spefific configuation union
}

table DataRequest {
  dstart:bool=true; // Start condition.
  dstart_alt:bool=false; // Alternate start condition.
  daddr:ubyte; // Device address (Bus Pirate automatically will set read/write bit)
  ddata:[ubyte]; // Data to write
  dreadbytes:uint32; // Number of bytes to read.
  dstop:bool=true; // Stop condition.
  dstop_alt:bool=false; // Alternate stop condition.
}

table DataResponse {
  ddata:[ubyte]; // Data read from device
  derror_message:string; // Error message if any.
}

union RequestPacketContents { StatusRequest, DataRequest}

table RequestPacket {
  version_major:uint8=0;
  version_minor:uint8=1;
  contents:RequestPacketContents;
}

union ResponsePacketContents { StatusResponse, DataResponse}

table ResponsePacket{
  version_major:uint8=0;
  version_minor:uint8=1;
  contents:ResponsePacketContents;
}

root_type ResponsePacket;
1 Like