crownstone / bluenet

Bluenet is the in-house firmware on Crownstone hardware. Functions: switching, dimming, energy monitoring, presence detection, indoor localization, switchcraft.
https://crownstone.rocks
91 stars 62 forks source link

Add versioning to IPC #185

Closed mrquincle closed 2 years ago

mrquincle commented 2 years ago

[1] It is perhaps sufficient to have just a minor update for a protocol update? [2] Naming must be so that it is very clear what is done where. The reason to have a protocol version for uploading microapps is to allow for different upload mechanisms without having to bump the general BLE protocol version. Updating the general BLE protocol version is very invasive.

vliedel commented 2 years ago

So the question is how to deal with IPC major version updates. The problem is that the version can impact 2 different things: the payload data (bootloader or microapp specific data), or the IPC header itself.

So a solution could be: add yet another version: the IPC header version. In that case, we sort of go back to having the data like:

struct ipc_header_t {
uint8_t version;
uint8_t index;
uint8_t dataSize;
uint16_t checksum;
};

struct ipc_bootloader_t {
uint8_t major;
uint8_t minor;
uint16_t dfuVersion;
...
};

struct ipc_microapp_t {
// Version of this struct.
uint8_t ipcMajor;
uint8_t ipcMinor;
microappCallbackFunc microappCallback;
...
};

Another solution would be to increase all majors to the same value on a breaking change of the IPC header.

I wouldn't let the caller make another call to getIpcData(), but instead let getIpcData() deal with the different IPC headers.

mrquincle commented 2 years ago

So you propose the thing that is implemented in this PR?

mrquincle commented 2 years ago

All concerns are addressed.

vliedel commented 2 years ago

Thanks :)