-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
fix and add sweeping compatibility #180
Conversation
* added compatibility with sweeping test * fix res_tester missing * fixed compatibility with Klipper < v0.12.0-239 * added documentation about sweeping mode
Reviewer's Guide by SourceryThis pull request introduces sweeping compatibility to ShakeTune by updating the resonance test methods to support both the old pulse-only method and the new sweeping method. It also refactors the resonance testing logic into a dedicated Class diagram showing resonance testing compatibility changesclassDiagram
class ResonanceTester {
+test [old]
+generator [new]
+probe_points [new]
}
class Test {
+min_freq
+max_freq
+accel_per_hz
+get_start_test_points()
}
class VibrationGenerator {
+min_freq
+max_freq
+accel_per_hz
}
ResonanceTester -- Test : has >
ResonanceTester -- VibrationGenerator : has >
note for ResonanceTester "Supports both old and new Klipper versions"
note for Test "Used in old Klipper (pre Dec 6, 2024)"
note for VibrationGenerator "Used in new Klipper (post Dec 6, 2024)"
State diagram for resonance testing modesstateDiagram-v2
[*] --> CheckKlipperVersion
CheckKlipperVersion --> OldVersion: has test attribute
CheckKlipperVersion --> NewVersion: has generator attribute
state OldVersion {
[*] --> PulseOnlyTest
PulseOnlyTest --> GetTestPoints
GetTestPoints --> ExecuteTest
}
state NewVersion {
[*] --> SweepingTest
SweepingTest --> GetProbePoints
GetProbePoints --> ExecuteTest
}
OldVersion --> [*]
NewVersion --> [*]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @Frix-x - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Summary by Sourcery
Update resonance testing to support Klipper's new sweeping method and maintain compatibility with the old pulse-only method.
New Features:
Tests: