Skip to content
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

Implement a userpsace low-level debug API for Tock 2.0 #345

Merged
merged 5 commits into from
Dec 1, 2021

Conversation

kupiakos
Copy link
Contributor

This resolves "Implement a LowLevelDebug driver" in #322.

Copy link
Collaborator

@jrvanwhy jrvanwhy left a comment

Choose a reason for hiding this comment

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

Looks good overall, my comments are mostly stylistic nits.

Cargo.toml Outdated Show resolved Hide resolved
apis/low_level_debug/src/lib.rs Outdated Show resolved Hide resolved
apis/low_level_debug/src/lib.rs Outdated Show resolved Hide resolved
apis/low_level_debug/src/lib.rs Outdated Show resolved Hide resolved
apis/low_level_debug/src/lib.rs Outdated Show resolved Hide resolved
apis/low_level_debug/src/lib.rs Outdated Show resolved Hide resolved
apis/low_level_debug/src/tests.rs Outdated Show resolved Hide resolved
apis/low_level_debug/src/tests.rs Outdated Show resolved Hide resolved
@jrvanwhy
Copy link
Collaborator

In general, libtock-rs APIs should not return CommandReturn directly, they should interpret it according to the documentation for the SyscallDriver they are interfacing to. LowLevelDebug is kind of unique in that its documentation says its commands should always return Success. Based on that, I think print_alert_code, print_1, and print_2 should just ignore the value returned from command.

What do you think?

@kupiakos
Copy link
Contributor Author

In general, libtock-rs APIs should not return CommandReturn directly, they should interpret it according to the documentation for the SyscallDriver they are interfacing to. LowLevelDebug is kind of unique in that its documentation says its commands should always return Success. Based on that, I think print_alert_code, print_1, and print_2 should just ignore the value returned from command.

What do you think?

My gut says no, keep it just in case, my brain says yes, errors should be silenced. I can't think of a realistic scenario where you would do something different if LowLevelDebug operations failed, since it exists for when other options fail.

Copy link
Collaborator

@jrvanwhy jrvanwhy left a comment

Choose a reason for hiding this comment

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

I just noticed that the low-level debug API is not exported from libtock2. We should export it, using something like the following (in libtock2/src/lib.rs):

pub mod low_level_debug {
    type LowLevelDebug = libtock_low_level_debug::LowLevelDebug::<runtime::TockSyscalls>;
    pub use libtock_low_level_debug::AlertCode;
}

This will require adding libtock_low_level_debug as a dependency of libtock2.

@kupiakos
Copy link
Contributor Author

kupiakos commented Dec 1, 2021

@jrvanwhy Added those exports. I considered having it be gated by a default-enabled feature in libtock2, but considering the low level debug API has no runtime overhead, I think it'll be fine.

@jrvanwhy
Copy link
Collaborator

jrvanwhy commented Dec 1, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 1, 2021

Build succeeded:

@bors bors bot merged commit 4372e78 into tock:master Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants