Skip to content

tracer: update public API for smoltcp::phy::Tracer to allow custom inspection and printing of packet #1076

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

Merged
merged 1 commit into from
Jul 19, 2025

Conversation

tomkris
Copy link

@tomkris tomkris commented Jul 19, 2025

Currently smoltcp::phy::tracer::Packet is not exported as public type, which leaves very limited options for using Tracer device: traced packets only can be directly printed "as-is" and only in limited context (inside the closure).

I'm updating API of Tracer device to provide access to internal fields of traced packet; this will enable implementation of custom printing or packet inspection functions

I renamed smoltcp::phy::tracer::Packet to smoltcp::phy::tracer::TracerPacket, following prefixing convention used by types declared in smoltcp::phy::pcap_writer

Copy link

codecov bot commented Jul 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.29%. Comparing base (a54589c) to head (72531a1).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1076      +/-   ##
==========================================
+ Coverage   79.62%   80.29%   +0.67%     
==========================================
  Files          81       81              
  Lines       24218    24303      +85     
==========================================
+ Hits        19284    19515     +231     
+ Misses       4934     4788     -146     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

buffer: &'a [u8],
medium: Medium,
prefix: &'static str,
direction: TracerDirection,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all of these can be pub fields. Also, the struct as a whole should have #[derive(Debug, Clone)] at least (probably Copy too)

Copy link
Author

Choose a reason for hiding this comment

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

Updated interface of TracerPkt to expose all fields publicly, removed now redundant constructor and accessor fields. Added Debug/Clone/Copy auto-derives.

@whitequark
Copy link
Contributor

It looks like there's no tests, can you add some?

@tomkris tomkris force-pushed the tracer branch 2 times, most recently from 566cf6d to 8d1a9ad Compare July 19, 2025 08:40
…spection and printing of packet

Currently `smoltcp::phy::tracer::Packet` is not exported as public type, which leaves very limited
options for using `Tracer` device: traced packets only can be directly printed "as-is" and only in
limited context (inside the closure).

I'm updating API of `Tracer` device to provide access to internal fields of traced packet; this will
enable implementation of custom printing or packet inspection functions

I renamed `smoltcp::phy::tracer::Packet` to `smoltcp::phy::tracer::TracerPacket`, following prefixing
convention used by types declared in `smoltcp::phy::pcap_writer`
@tomkris
Copy link
Author

tomkris commented Jul 19, 2025

Thank you for taking a look Catherine,

Added unit test for the Tracer device implementation and for Display implementation of TracerPacket.
Also added standard derives to TracerDirection

Copy link
Contributor

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@whitequark whitequark added this pull request to the merge queue Jul 19, 2025
Merged via the queue into smoltcp-rs:main with commit 8203207 Jul 19, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants