Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add APDU debugging using a trace api similar to httptrace #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

areese
Copy link
Contributor

@areese areese commented Feb 19, 2024

@ericchiang Please let me know what you think of this way of tracing.
I need to sign the commits, I don't have the keys right now, and I need to try this out, I don't have the harness.

Copy link
Collaborator

@ericchiang ericchiang left a comment

Choose a reason for hiding this comment

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

Rather than using contexts, I would recommend the exported API look something:

type ClientTrace struct {
    Transmit func(req []byte)
    TransmitResult func(...)
}
type Client struct {
    Trace *ClientTrace
}
func (c *Client) Open(*Yubikey, error) { ... }

Dropping the WithClientTrace methods and use of contexts

@areese
Copy link
Contributor Author

areese commented Feb 20, 2024

Rather than using contexts, I would recommend the exported API look something:

type ClientTrace struct {
    Transmit func(req []byte)
    TransmitResult func(...)
}
type Client struct {
    Trace *ClientTrace
}
func (c *Client) Open(*Yubikey, error) { ... }

Dropping the WithClientTrace methods and use of contexts

I think that's way better. I didn't think of that path, and just added context.

A ClientTrace struct can be set on a YubiKey object,
or a piv.Client struct can be created and Open called against that.
@areese areese force-pushed the add-adpdu-debugging branch from f044341 to d480045 Compare February 20, 2024 02:56
@areese areese mentioned this pull request Feb 20, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants