-
Notifications
You must be signed in to change notification settings - Fork 126
Tip-aware cLLD probing start_pos_search and safety validation
#804
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
base: main
Are you sure you want to change the base?
Tip-aware cLLD probing start_pos_search and safety validation
#804
Conversation
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.
Pull request overview
This PR refactors the capacitive liquid-level detection (cLLD) probing logic to make the tip length calculation tip-aware and adds comprehensive safety validation. The primary purpose is to ensure that the Z-drive start position accounts for the actual tip length rather than using a fixed value, preventing potential collisions or invalid search positions.
Key Changes:
- Made
start_pos_searchoptional with automatic computation based on tip length when not provided - Added precondition checks including tip presence validation and channel index bounds checking
- Enhanced documentation with detailed docstring describing all parameters, behavior, and exceptions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
rickwierenga
left a comment
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.
this PR makes move_z_drive_to_liquid_surface_using_clld depend on C0 (through self.request_tip_presence and self.request_tip_len_on_channel), meaning it can no longer be run in parallel. running this function in parallel was the reason for splitting it out of clld_probe_z_height_using_channel in #742
I don't think it is possible to have these c0 features and run it in parallel, meaning the caller will have to perform checks instead of this function. maybe we should make it private? _move_z_drive_to_liquid_surface_using_clld
it is important that we can run this in parallel for probing multiple liquid height simultaneously.
|
Thank you for catching this @rickwierenga ! I have excised the |
| if start_pos_search is None: | ||
| start_pos_search = safe_head_top_z_pos - tip_len + fitting_depth |
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.
why is this implicit behaviour good?
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.
Can you please elaborate what you mean?
No description provided.