Open rmustacc opened 1 month ago
Hmmm. Does the firmware need to parse the serial number, or can we manipulate it as an opaque blob? I'm a little surprised we're parsing it now (but I didn't write this code). If we could insulate it from any future changes, that seems nice.
I think it probably is helpful to have a difference between the three different encoded fields (CPN, Rev, and Serial), but I suspect that it probably can treat it the contents mostly as an opaque thing. Though I guess it's always possible in the future that for certain parts you'll need to look at the rev to know how to handle some change that isn't on the baseboard (or the baseboard didn't end up with the compat version change for some reason). But I haven't audited all consumers of the structure yet.
The "barcode" encoding which decided not to break apart separately in the FRUID rom is a thing that's not exposed anywhere higher up and the other interfaces we have which have separate CPN, Revision, and Serial fields, are something that we would like to continue honoring if possible. I realize I'm not sure if you're referring to treating the whole string as opaque, or just the contents of the fields that it separates with the :
characters.
I realize I'm not sure if you're referring to treating the whole string as opaque, or just the contents of the fields that it separates with the : characters.
The former if possible, the latter if not. So probably the latter. :-)
If we wind up needing to parse and switch on revisions in the future, we can implement it then, IMO. For handling dynamic lengths, it looks like removing the ==
check on the length might suffice? That'd be nice.
In the OXV2 barcode format, there are a series of
:
separated fields that are used to distinguish between the part number, revision, and serial number respectively. Hubris parses this VPD encoded line using the internaloxide-barcode
crate.While this crate does split on the
:
characters, it is making internal assumptions about the number of characters that will and won't be present in various parts here. See for example, some of the logic in VpdIdentity parse function.The changes that are happening with these is that Terra CPNs are going to be shorter and the v2 serial numbers are also shorter. From looking at where these are all consumed in the system through both the inventory data and the SP status messages, it seems that those all are using larger fields for the serial and part numbers, such that shorter ones should be fine. The main issue that we'll need to sort through is how we want to change the parsing logic and the
VpdIdentity
structure.Thinking out loud, it seems likely that we can just retain the existing structure and mostly change the logic in this case to deal with dynamic lengths and perhaps error if the length is larger than the current set aside field, but otherwise zero pad the rest.