Have Control communicate a version

[rrbutani]

what

the problem

Eventually as we continue development we're going to want to make changes to Control. The opens the door to mismatches between the version of the Control trait host programs (like the tui) use and versions used by the device.

The Control trait's RPC mechanism is, at it's core, just based on serde Serialize/Deserialize implementations for Request and Response types that are 1-to-1 with the Control trait. These implementations are not forwards/backwards compatible like other encoding mechanisms (like Protobufs) are which means you can run into painful issues where, for example, a board speaks a newer version of Control with added methods and thus completely different numbering for the variants on RequestMessage.

I maintain that choosing a low overhead encoding mechanism like postcard instead of something with some schema information (like JSON) is the right choice for embedded but this is definitely a failure mode we still want to guard against.

a solution

In cases where there's mismatch between the host/device I think it's sufficient to have the host error and to tell the user to update their device/host (depending on which has the older version).

(Later, if desired, we can look into having the host support multiple versions of Control and selecting the right one at runtime)

We can do this by adding a control_protocol_version method to Control that returns a tuple ((u16, u16)) for major version and minor version.

This function should be default impl'd; the idea is that the version of lc3-traits used to build a particular host program or device program determines the protocol version (i.e. this is not something Control implementors need to specify manually).

The idea is that, on startup, the host should ask the device for this version tuple and then decide if it can continue communication or not.

This requires host and device Control implementations that do not have the same version to still be able to exchange at least this message. We can achieve this by requiring that this forever be the first message in RequestMessage and ResponseMessage and having a regression test that ensures this.

For versioning, major version changes to the Control version should coincide with major version changes to the lc3-traits crate. Minor version changes (i.e. adding new functions to the end of Control that changes RequestMessage and ResponseMessage in a backwards compatible way (i.e. does not change the enum variant encoding)) do not need to correspond to a semver break but should indeed be backwards compatible changes.

I think it's hard for us to ensure changes follow this policy but we can add some added friction that makes it obvious to developers when they're making a change that they need to abide by this policy; i.e. having a const assert on the Control version and number of methods, etc. If we end up making a macro that derives RequestMessage and ResponseMessage from Control (as is the plan) this is doable.

All together:

const CONTROL_MAJOR_VERSION: u16 = 1;
const CONTROL_MINOR_VERSION: u16 = 0;

#[lc3_macros::rpc(mod = rpc, req = RequestMessage, resp = ResponseMessage)]
trait Control {
  fn version(&self) -> (u16, u16) { (CONTROL_MAJOR_VERSION, CONTROL_MINOR_VERSION) }

  // ...
  // <sniped>
}

const VERSION_TABLE: &[] = &[
  /* major version 0 */ MAJOR_VERSION_0_INFO,
];

// sizes
const MAJOR_VERSION_0_INFO: &[] = &[
  /* (0, 0) */ (/* num messages */ 20) 
];

/// Guard against accidental changes to RPC!
const_assert!({
  rpc::NUM_MESSAGES == VERSION_TABLE[CONTROL_MAJOR_VERSION][CONTROL_MINOR_VERSION]
});

// can have more sophisticated tracking for backwards compat (i.e. each version can have a message listing, each newer version for a particular major version must have the same list in the same order but can have extensions, etc.) if we want
// the above is sufficient to get developers attention that there's a process they need to follow though

#[test]
fn first_message_is_version_message() {
  assert!(unsafe { /* init RequestMessage from index */ } == rpc::RequestMessage::GetVersion)
}

steps

• [[ steps ]]

TUI support is the last step, should make a corresponding issue for that

[[ What steps are needed to implement this improvement? ]]

where

branch: feat-control-trait-versioning

open questions

unrelated but have we settled on having a de facto encoding method? i.e. are we going to force all board impls to use postcard or do we want to make that customizable per device impl too? (i.e. do we want the device info abstractions described in ut-utp/tui#11 to grow ways of specifying the whole transport?)

I'm leaning towards just requiring postcard (or some fixed encoding scheme).

[rrbutani]

eerie coincidence but this, from a few hours ago, is very relevant: fork with an implementation