Introduce humility-vpd-lib#670
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has totally checked 102 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 100 | 2 | 0 | 0 |
Click to see the invalid file list
- humility-vpd-lib/examples/list.rs
- humility-vpd-lib/examples/read.rs
|
|
||
| pub enum VpdTarget { | ||
| Device(usize), | ||
| Loopback(fs::File), |
There was a problem hiding this comment.
I'm waffling if we should just remove the loopback testing code
There was a problem hiding this comment.
I was confused why all these functions needed to pass a &mut VpdTarget; it's because functions mutate the fs::File inside the Loopback variant. That weirdness is another point in favor of just removing it.
There was a problem hiding this comment.
I just removed it but kept the VpdTarget (maaaybe that should just be a straight usize?)
| let probe = std::env::var("HUMILITY_PROBE").unwrap(); | ||
|
|
||
| let hubris = RawHubrisArchive::load(&hubris).unwrap(); | ||
| let hubris = HubrisArchive::load(hubris, None).unwrap(); |
There was a problem hiding this comment.
This suggests that we're missing a HubrisArchive::from_archive(path: FilePath). How about adding that function, then removing the pub export of RawHubrisArchive?
|
|
||
| let core = &mut *humility_probes_core::attach_to_chip( | ||
| &probe, | ||
| Some("STM32H753ZITx"), |
There was a problem hiding this comment.
Can we get the chip from the manifest?
There was a problem hiding this comment.
#675 we can make this one step with an extension trait
| err: std::io::Error, | ||
| }, | ||
| #[error("tlvc: {0:?}")] | ||
| Tlvc(TlvcReadError<core::convert::Infallible>), |
There was a problem hiding this comment.
Opened oxidecomputer/tlvc#11 to make this better
| /// across hubris releases | ||
| pub ndx: usize, | ||
| /// Raw i2c data about the device | ||
| pub device: HubrisI2cDevice, |
There was a problem hiding this comment.
Vibes: this is a moderately large value type. When I was poking at humility validate-as-a-library, I found it was possible to borrow instead (e.g. add a lifetime to VpdEntry and have pub device: &'a HubrisI2cDevice); check and see how that goes?
| let mut offset = 0; | ||
|
|
||
| // Do we want to make this an argument for optionally not having | ||
| // a progress bar? |
There was a problem hiding this comment.
We'll eventually want something like trait HumilityProgress, but that doesn't need to happen now...
|
Open question: we will probably grow more humility-lib crates. Do we want to keep them all at the root for now and make cleanup a problem for Future Us or try some organization now? |
mkeeter
force-pushed
the
mkeeter/subcommand-enum
branch
2 times, most recently
from
9474cbb to
8c8ab59
Compare
For library functions this is a nice shortcut if we don't need to directly manipulate the `RawHubrisArchive`
This saves some steps for humility as a library
| /// Reading out the VPD can be very slow and can be omitted | ||
| pub data: VpdData, |
There was a problem hiding this comment.
I don't love this but it takes almost 15 seconds to do a list when reading all the VPD
mkeeter
force-pushed
the
mkeeter/subcommand-enum
branch
from
4c980ca to
4665ca1
Compare
mkeeter
force-pushed
the
mkeeter/subcommand-enum
branch
3 times, most recently
from
f49aa94 to
20d913b
Compare
2 participants
It's useful to be able to call VPD functions from Rust code