Fix touchdown detection logic in SwingTrajectoryController#36
Open
amlansahoo07 wants to merge 1 commit intoiit-DLSLab:mainfrom
Open
Fix touchdown detection logic in SwingTrajectoryController#36amlansahoo07 wants to merge 1 commit intoiit-DLSLab:mainfrom
amlansahoo07 wants to merge 1 commit intoiit-DLSLab:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR corrects the touchdown detection in SwingTrajectoryController by switching from lift-off to touchdown logic, adding stateful timing, and enforcing a short stability delay before triggering optimization.
- Corrected core event detection from swing-to-stance (touchdown) rather than stance-to-swing
- Introduced internal state tracking for full-stance timing and recent touchdown flags
- Added a stability delay (based on simulation
dt) to prevent premature optimization triggers
Comments suppressed due to low confidence (2)
quadruped_pympc/helpers/swing_trajectory_controller.py:147
- [nitpick] New multi-stage touchdown timing logic is complex and critical for gait optimization. Please add unit tests (e.g., mocking contact transitions and varying
dt) to verify correct behavior across edge cases.
def check_touch_down_condition(self, current_contact, previous_contact):
quadruped_pympc/helpers/swing_trajectory_controller.py:200
- The indentation for
touch_down = 1appears misaligned with its conditional block. Ensure it is nested underif self._recent_touchdown:so it only executes when intended and avoid a syntax error.
touch_down = 1
| self._full_stance_start_time = current_time | ||
| else: | ||
| # Check if we've been in full stance long enough (e.g., 50ms for stability) | ||
| stability_delay = 25 * dt |
There was a problem hiding this comment.
[nitpick] The hardcoded multiplier 25 is a magic number. Consider extracting it into a named constant or configuration parameter (e.g., min_stability_steps) to improve clarity and make it easier to adjust.
Suggested change
| stability_delay = 25 * dt | |
| stability_delay = self.MIN_STABILITY_STEPS * dt |
Collaborator
|
Thanks Amlan. I'm planning to review this PR this week. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The check_touch_down_condition() method in SwingTrajectoryController had incorrect logic that detected lift-off events instead of touchdown events. This caused step frequency optimization to fail with slow gaits, particularly crawl patterns at 0.5 Hz step frequency.
Implemented a robust three-stage touchdown detection algorithm:
This fix enables proper step frequency optimization for crawl gaits and improves the reliability of gait adaptation across all locomotion patterns.