Skip to content

Introduce extension trait for attaching #675

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.

Sign up for GitHub

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

Open
labbott wants to merge 2 commits into
base: master
Choose a base branch
from
+13 −0
Open

Introduce extension trait for attaching #675

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12560ba
Introduce extension trait for attaching
labbott May 21, 2026
f156191
change the name
labbott May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions humility-probes-core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,16 @@ pub fn attach_for_flashing(
) -> Result<Box<dyn Core>> {
attach_to_chip(probe, Some(chip), speed_khz)
}

/// Extension trait to make it easier for libraries to attach
/// to chips. It is encourageed to extend this trait as
Copy link
Copy Markdown

@jamesmunns jamesmunns May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/// needed to make developer's lives easier.
pub trait HubrisAttach {
fn attach_to_chip(&self, probe: &str) -> Result<Box<dyn Core>>;
Copy link
Copy Markdown

@jamesmunns jamesmunns May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

impl HubrisAttach for humility::hubris::HubrisArchive {
fn attach_to_chip(&self, probe: &str) -> Result<Box<dyn Core>> {
Copy link
Copy Markdown

@jamesmunns jamesmunns May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@labbott labbott May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

attach_to_chip(probe, self.chip().as_deref(), None)
}
}
Loading