Introduce extension trait for attaching

[labbott]

This saves some steps for humility as a library

[mkeeter]

Hmmm, should this be called attach_to_chip or attach_probe to be less ambiguous (especially if we're going to add more methods in the future)?

[labbott]

I'll go with attach_to_chip to match

[mkeeter]

Sorry for being pedantic, but now that you've updated it I've convinced myself that attach_probe is the Right Name:

• The single &str argument is a probe name, not a chip name
• It returns a boxed-up ProbeCore

[jamesmunns]

I have a personal bias against "extension traits", mostly because they don't show up well in docs and can be a little tedious to use.

That being said: I don't totally understand the intended use, and if the intent is that 3rd party (at least "not humility") libraries are going to implement and use it, rather than humility implementing this interface for external libraries, I probably wouldn't call this an "extension trait" anyway!

It might be worth documenting when and why folks would want to impl this trait on their types, and what it does/doesn't get them?

[jamesmunns]

Suggested change

	/// to chips. It is encourageed to extend this trait as
	/// to chips. It is encouraged to extend this trait as

[jamesmunns]

Maybe document what this is supposed to do and when this should/shouldn't fail?

[jamesmunns]

Just a check, do we want this to be Box<dyn Core> instead of being an assoc type? (if we're already using it as dyn everywhere in existing code, the answer is probably "yes")

Perf wise it probably doesn't matter, but just checking because sometimes dyn Trait is "a land of contrasts", as Eliza would say.

[labbott]

yeah we return dyn Core everywhere (see the other attach functions in https://github.com/oxidecomputer/humility/blob/8f0fb59e4c0d7453f1382a1700915e6c59cfe07e/humility-probes-core/src/lib.rs)

[labbott]

I have a personal bias against "extension traits", mostly because they don't show up well in docs and can be a little tedious to use.

That being said: I don't totally understand the intended use, and if the intent is that 3rd party (at least "not humility") libraries are going to implement and use it, rather than humility implementing this interface for external libraries, I probably wouldn't call this an "extension trait" anyway!

It might be worth documenting when and why folks would want to impl this trait on their types, and what it does/doesn't get them?

Maybe I'm using the term "extension trait" incorrectly. This isn't intended to be implemented by anyone and is more a hack to be able to have another impl. It's designed to make it easier to attach to a probe with a Hubris archive. In my experiments with humility as a library we have to do

let hubris = HubrisArchive::load_from_path(&hubris).unwrap();
let core = &mut *humility_probes_core::attach_to_chip(
        &probe,
        hubris.chip().as_deref(),
        None,
    )
    .unwrap();

Which is a bit ugly. My thought was that it would be much more natural to just do let core = &mut *hubris.attach(&probe).unwrap(); instead. Today I realized some more checks I would like to add as well besides just wrapping the attach so I definitely think something like this would be useful.

I'm not super attached to the trait (pun not intended) so maybe a static function would be cleaner?

/// calls attach_to_chip with parameters from the hubris archive
pub fn attach_via_hubris_archive(archive: &HubrisArchive) -> Result<...>

[labbott]

Looking at all this code makes me want to rework attaching because it's not well documented but I am trying to avoid finding yet another Yak to shave (there are so many in humility)