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

Firmware Git Hash #17

Open
bruno-f-cruz opened this issue Dec 8, 2023 Discussed in #16 · 4 comments · May be fixed by #38
Open

Firmware Git Hash #17

bruno-f-cruz opened this issue Dec 8, 2023 Discussed in #16 · 4 comments · May be fixed by #38
Assignees
Labels
under review An issue that is undergoing pr review under testing Someone is having testing this feature

Comments

@bruno-f-cruz
Copy link
Member

Discussed in #16

Originally posted by Poofjunior September 21, 2023
Hi folks,

Is there any reason Harp stores firmware revision numbers instead of a git hash?

The shortened git hash is

  • 8 bytes long
  • pins the exact version of the code and can be restored with git checkout <hash>
  • can be generated at compile-time and inserted into the code with a #define macro.

Perhaps this is a change that could be worth considering in the 32-bit protocol?

@Poofjunior
Copy link

Adding to this.

Main benefits are:

  • unambiguous and highly specific (Think patch in major, minor, patch semantic versioning.)
  • enables recovering the firmware state down to the commit level.
  • storing the git hash on the device can be automated at compile time and is not subject to user/developer error in bookkeeping

It's likely that Harp boards will be occasionally repurposed with custom firmware specific to a one-off experiment. In fact, this has already happened where Behavior boards in our Ephys rigs run special firmware to emit a clock signal on one of the GPIO pins for synchronization with Neuropixels Probes traces. In we put a rig into a one-off "Goldilocks" state without having a way to recover that state, we don't give ourselves an easy path forward to duplicating experiment results to other rigs. Recording the git hash always gives us a highly-specific way of recovering the device state.

@glopesdev
Copy link
Collaborator

glopesdev commented Dec 13, 2023

I very much agree with the spirit of the proposal, but fixing the use of Git hashes in the standard does not seem like a good idea to me. The main reason is that it ties the two technologies together too much, for example in the past I had to move bonsai from Mercurial to Git and I can only thank that I didn't do any similar decision, otherwise it would have been even more painful.

It also somehow feels clunky to me having to refer to Git concepts to explain the Harp protocol.

My counter-proposal would be to consider having some kind of loosely defined "tag" register that can be used by firmware developers to place whatever special ID they want, a bit like prerelease strings in semantic version. That way different groups might use different strings at different times to identify these "special builds".

Similar to semantic versioning, the only expectation would be that these strings are used for "prereleases" that are not more explicitly versioned in the standard otherwise.

@bruno-f-cruz
Copy link
Member Author

bruno-f-cruz commented Jan 11, 2024

Feedback from SRM:

  • @glopesdev : "If anything other than 0 is written to the register, we assume its a preview release". However @Poofjunior pointed out that this invalidates using the register as a way to keep track of a git hash for full releases.
  • @glopesdev pointed out that this should not be tied to a particular version control platform and instead be used for what the developer needs (i.e. a tag). This Tag can be automatically or manually generated, up to the developer.

Decisions to be made:

  • Register name: Tag (Unanimous)
  • Register Number: To be determined
  • Register Size: It would be nice to have a string that allows a small github hash (8bytes?)
  • Register Access: The register should be static and read-only. (Unanimous)

@bruno-f-cruz
Copy link
Member Author

Feedback from 29022024:

Register name: Tag
Register Number: 17
Register Size: uint8[8]
Register Access: Read-only as per protocol specification

@bruno-f-cruz bruno-f-cruz added the under review An issue that is undergoing pr review label Feb 29, 2024
@bruno-f-cruz bruno-f-cruz self-assigned this Feb 29, 2024
@bruno-f-cruz bruno-f-cruz linked a pull request Feb 29, 2024 that will close this issue
@bruno-f-cruz bruno-f-cruz added the under testing Someone is having testing this feature label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under review An issue that is undergoing pr review under testing Someone is having testing this feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants