Move more Core functions to ProbeCore#700
Conversation
|
I had to do a bit of extra wrangling to get the probeless build working; it now skips subcommands which are probe-only, both in the |
I had hoped to be able to drop this after the probe-rs update but so long as we need to keep CMSIS DAP v1 around we will still need these hacks :( |
mkeeter
force-pushed
the
mkeeter/remove-more-core-functions
branch
from
547b59b to
a8aa5cc
Compare
labbott
left a comment
labbott
left a comment
There was a problem hiding this comment.
minor fixup but LGTM otherwise
| pub fn vid_pid(&self) -> Option<(u16, u16)> { | ||
| Some((self.vendor_id, self.product_id)) | ||
| } |
There was a problem hiding this comment.
I think we can drop the Option here now
|
I also think we can move |
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
looks good to me overall, with some minor nits
| #[cfg(not(feature = "probes"))] | ||
| { | ||
| Err(anyhow!( | ||
| "probes feature is missing; ip address or dump is required" |
There was a problem hiding this comment.
similarly, maybe:
| "probes feature is missing; ip address or dump is required" | |
| "humility was compiled without the \"probes\" feature; \ | |
| an IP address or dump is required" |
| Err(anyhow!( | ||
| "probes feature is missing; ip address is required" | ||
| )) |
There was a problem hiding this comment.
turbo nit: maybe:
| Err(anyhow!( | |
| "probes feature is missing; ip address is required" | |
| )) | |
| Err(anyhow!( | |
| "humility was compiled without the \"probes\" feature; \ | |
| an IP address is required" | |
| )) |
or something? i think "probes feature is missing" is a little bit unclear for the user who may not be aware that this is specifically referring to a compile-time feature flag rather than some CLI arg you forgot to provide or something...
| pub fn vid_pid(&self) -> Option<(u16, u16)> { | ||
| Some((self.vendor_id, self.product_id)) | ||
| } |
There was a problem hiding this comment.
This is maybe a suggestion better saved for a follow-up change, since I'm sure it will break some stuff, but I really wish that this was
pub struct VidPid {
pub vid: u16,
pub pid: u16,
}rather than a tuple, so it's impossible to get them backwards (since we've had at least one bug where we did that).
There was a problem hiding this comment.
This is easy enough; I also added a Display implementation that prints vid:pid with the expected formatting.
mkeeter
force-pushed
the
mkeeter/remove-more-core-functions
branch
from
a8aa5cc to
26cd75e
Compare
That was easy enough to do that I just did it here as well |
mkeeter
force-pushed
the
mkeeter/remove-more-core-functions
branch
from
e699fc9 to
242ae51
Compare
mkeeter
force-pushed
the
mkeeter/remove-more-core-functions
branch
from
242ae51 to
6beb7b6
Compare
vid_pid,step, andwrite_regare only available onProbeCore. This PR removes them from theCoretrait, adjusting signatures where necessary to force the concrete type.