-
-
Notifications
You must be signed in to change notification settings - Fork 150
Add dict to types of CapabilityTypes #1286
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: dev
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #1286 +/- ##
=======================================
Coverage 94.62% 94.62%
=======================================
Files 149 149
Lines 5856 5857 +1
Branches 350 350
=======================================
+ Hits 5541 5542 +1
Misses 253 253
Partials 62 62 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 extends the auto-empty capability for the fd60kt device to support a new "MANUAL" frequency mode. The change allows the types field in CapabilityTypes to be either a tuple (existing behavior) or a dictionary mapping frequency values to their command parameters.
Key changes:
- Added
MANUALfrequency option to theauto_empty.Frequencyenum - Extended
CapabilityTypesto accept dict format for type definitions with associated parameters - Updated fd60kt device configuration to use dict-based types with enable/frequency parameters for each mode
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| deebot_client/events/auto_empty.py | Added MANUAL frequency enum value |
| deebot_client/capabilities.py | Extended types field to support dict format alongside existing tuple format |
| deebot_client/hardware/fd60kt.py | Converted auto_empty types from tuple to dict format with enable/frequency parameters for each mode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #1286 will not alter performanceComparing Summary
|
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
For the DEEBOT T50 OMNI (fd60kt), both
enableandfrequencymust be specified simultaneously for the auto_empty command to be accepted. However, the current select entity only allows sending a single parameter, resulting in an error.To pass multiple parameters to integration with a single selection, I modified the type to allow both
dictandtuple. (due to the number of changes required, I didn't replace thetupleswithdict) Since the integration side remains unchanged, this modification alone does not alter the behavior. For example, the following code will behave identically in either case.options_fn=lambda cap: [get_name_key(freq) for freq in cap.types]I think adding test is sufficient in integration, but if necessary, specific instructions would be helpful.
I'm not sure if this is the correct approach as code... Any advice would be appreciated.