Skip to content

Add set_slave_only method to PtpInstance#801

Merged
davidv1992 merged 1 commit intopendulum-project:mainfrom
bhagen55:main
Mar 11, 2026
Merged

Add set_slave_only method to PtpInstance#801
davidv1992 merged 1 commit intopendulum-project:mainfrom
bhagen55:main

Conversation

@bhagen55
Copy link
Contributor

It can be useful to configure a port's slave_only configuration on the fly. For example, a port might need to start out in slave_only = true while it waits for it's PHC clock to synchronize for hardware timestamping to be reliable. After health checks pass, it can set slave_only = false and become a master if applicable.

I made a small change to add a set_slave_only function to the default dataset, but that wasn't quite enough since the code currently assumes slave_only is set during init and won't change. The largest change is in set_recommended_port_state which will no longer error out if a port is a master with slave_only set. Instead, it will be forced to PortState::Listening like other port states already are.

I'll admit I don't have a wide enough knowledge of the codebase to know if this is all that is needed for this change to work. I added a few unit tests that seem to show it does, but my confidence level isn't super high still. I did test this practically on the bench and it appears to function as intended.

Resolves #799

This will prevent a port from becoming a master, or cause a port that is currently a master to be demoted during the next BMCA decision.
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 98.64865% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.82%. Comparing base (fe98d5d) to head (e6aee01).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
statime/src/port/bmca.rs 98.38% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #801      +/-   ##
==========================================
+ Coverage   63.64%   63.82%   +0.18%     
==========================================
  Files          62       62              
  Lines        7950     8020      +70     
==========================================
+ Hits         5060     5119      +59     
- Misses       2890     2901      +11     
Flag Coverage Δ
fuzz 14.68% <0.00%> (-0.04%) ⬇️
fuzz-message_sound 14.68% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@davidv1992
Copy link
Member

Thank you for contributing this. This looks like a very good idea, and with how the codebase works this is the correct approach.

@davidv1992 davidv1992 added this pull request to the merge queue Mar 11, 2026
Merged via the queue into pendulum-project:main with commit 4451b63 Mar 11, 2026
4 checks passed
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.

Allow adjustment of slave_only parameter on the fly

2 participants