-
-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
SingleRegisterAccess: Support unavailable regs #107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't mind those failing clippy tests - I'll need to go in and fix those up myself prior to pushing out the next minor release.
[...] but it would probably benefit from introducing an
Option
instead of the C-ish approach of giving special meaning to0
I completely agree. This approach is fine for now, but once this PR goes in, we should open a tracking issue to change the API in 0.7
to use an Option
instead.
Some general comments:
- This new functionality needs to be documented. Please update the
SingleRegisterAccess::read_register
docs accordingly. - Please include the diff of
./example_no_std/check_size.sh
as part of your validation - Please update the
armv4t
example in-tree to include a new register that does not get collected (so this code path has an example + gets tested), and then update the validation section with GDB output from the example + the packet trace log.
fwiw, these are all part of the PR checklist, and are there for a reason. Please don't ignore them, and stick to the PR checklist
Sorry for forgetting about documentation, that's fixed. And thanks for explaining what's expected from the armv4t test, I've added support for an " Note that
Should it be fixed? |
All good 👍 Ahh, yeah, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aside from this one comment, LGTM.
(oh, and it looks like rustfmt is complaining)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder that you'll still need to include the output of example_no_std/check_size.sh
in your PR description. It might seem trivial, but one of gdbstub
's stated goals is to have as small of a footprint on embedded systems as possible, so keeping an eye on the minimal configuration code size is a necessity in all PRs.
Enable an implementation of SingleRegisterAccess to signal to gdbstub that a read register is recognized (as per RegId::from_raw_id()) but is unavailable by reporting a 0-byte read. In that case, make gdbstub report to the GDB client the state of the unavailable register by following the registers packet protocol: > [...] the stub may also return a string of literal `x`s in place of > the register data digits, to indicate that the corresponding register > has not been collected, thus its value is unavailable. Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
Output of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Thanks for putting up with all my back-and-forth, this looks ready to rock 🪨
Let me know if you're in a rush to use this feature, and I can push out a 0.6.3
release at some point in the next week :)
Description
Make gdbstub report to the GDB client the state of an unavailable (but valid) register by following the registers packet protocol:
API Stability
Option
instead of the C-ish approach of giving special meaning to0
Checklist
rustdoc
formatting looks good (viacargo doc
)examples/armv4t
withRUST_LOG=trace
+ any relevant GDB output under the "Validation" section below./example_no_std/check_size.sh
before/after changes under the "Validation" section belowValidation
GDB output
examples/armv4t
then
makes the GDB client learn about the
Unavailable
register through the XML:Then requesting
shows the appropriate request (
0x1c
= 28) and response (xxxxxxxx
= 4 unknown bytes):which the GDB client reports:
Before/After `./example_no_std/check_size.sh` output
Before
After