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

Check Firmware Compatibility #77

Open
daniel-j-h opened this issue Apr 30, 2017 · 7 comments
Open

Check Firmware Compatibility #77

daniel-j-h opened this issue Apr 30, 2017 · 7 comments
Labels

Comments

@daniel-j-h
Copy link
Collaborator

From #70 (comment), quoting @dcyoung:

Correct, the SDK will no longer work with firmware v 1.0 which did not include the "MZ" command, among other things.

We should provide a way for the user to check if the sweep-sdk is compatible with the hardware device.

Or at least note it down in the readme and tag releases (i.e. git tag `vx.y.z)) and then provide changelog'ish summaries.

@daniel-j-h
Copy link
Collaborator Author

Ah I've just seen 68042b7 - perfect.

The release process should be as follows:

@daniel-j-h
Copy link
Collaborator Author

Here's what we need to implement on the SDK side:

@dcyoung
Copy link

dcyoung commented May 1, 2017

Definitely agree!

I have verified that the IV command is expected to be consistent across all future firmware revisions, so we are OK to go that route.

Implementing the IV command and extracting the firmware version should be simple. Where should we perform the check? During device construction, after the stop_scanning() call?

@daniel-j-h
Copy link
Collaborator Author

We need a device object so the compatibility check should happen in the construct function.

Probably here.

@dcyoung
Copy link

dcyoung commented May 12, 2017

What scheme should we use to determine the compatibility between firmware and libsweep versions?

As I see it, we could:

  1. Include definitions for FIRMWARE_VERSION_MAX and FIRMWARE_VERSION_MIN in the cmake config file.... then write a simple method to check if the parsed firmware of the device falls between the two. This protects against both outdated firmware or outdated libsweep installations. However, we would have to increment the version of libsweep just to increment the MAX value in the cmake config file. It seems unnecessary to make a new release of libsweep if the updated firmware is fully compatible with the current version.

  2. Only define a FIRMWARE_VERSION_MIN, and assume that users will use the latest version of libsweep that supports their firmware. This is simple, and doesn't require that we increment libsweep for each firmware release, unless necessary. However, the firmware check will only protect against outdated firmware... and not against outdated libsweep.

@daniel-j-h what do you think?

@daniel-j-h
Copy link
Collaborator Author

How's the firmware versioned? Major version bump for protocol incompatibilities? If so I would add

SWEEP_PROTOCOL_VERSION_MAJOR
SWEEP_PROTOCOL_VERSION_MINOR
SWEEP_PROTOCOL_VERSION

in config.h.in similarly to the ABI versioning. Then in the sweep.h API expose

SWEEP_API bool sweep_is_firmware_compatible(sweep_device_s device, sweep_error_s* error);

and implement a check for major protocol version equality.

Maybe expose a function in the API to receive the firmware's firmware major and minor version, which you then can call in the firmware check - maybe not. Not sure if the firmware version could be interesting for users other than for doing a compatibility check.

@dcyoung
Copy link

dcyoung commented May 15, 2017

The IV command returns both a firmware version and a protocol version which are updated independently. Both use a MAJOR.MINOR scheme. Unfortunately, the IV command only uses a single ASCII digit for the MAJOR and MINOR versions. By that I mean, the IV command can return a protocol or firmware version of X.X but not XX.X or X.XX. This could be rectified by changing the IV command output, but that would break compatibility. I doubt many users have built logic around the IV command yet, so this shouldn't be a big deal breaker.

It is possible that a firmware version would cause incompatibilities. For example, a firmware update could increase the wait time before performing a calibration routine... which might be incompatible with a timeout used in libsweep.

As for the protocol version... if existing parts of protocol are deleted or modified, there could be incompatibilities with libsweep. But additions to the protocol shouldn't break compatibility. From now on we will increment the protocol MAJOR for changes or deletions to parts of the protocol. We will increment the protocol MINOR for additions to the protocol that do not break compatibility.

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

No branches or pull requests

2 participants