Skip to content
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

Use standard identifiers for roborock #1729

Merged
merged 6 commits into from
Feb 11, 2023

Conversation

starkillerOG
Copy link
Contributor

Follow the new standard set in #1724 by @rytilahti

@starkillerOG starkillerOG changed the title use id instead of type roborock use id instead of type Feb 10, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2023

Codecov Report

Merging #1729 (2bd50f7) into master (b4b7f1a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #1729   +/-   ##
=======================================
  Coverage   81.93%   81.93%           
=======================================
  Files         196      196           
  Lines       18051    18054    +3     
  Branches     3863     3863           
=======================================
+ Hits        14790    14793    +3     
  Misses       2972     2972           
  Partials      289      289           
Impacted Files Coverage Δ
miio/identifiers.py 100.00% <100.00%> (ø)
miio/integrations/roborock/vacuum/vacuum.py 64.83% <100.00%> (+0.06%) ⬆️
...o/integrations/roborock/vacuum/vacuumcontainers.py 85.92% <100.00%> (+0.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rytilahti
Copy link
Owner

rytilahti commented Feb 10, 2023

So I'm not yet sure what's the best approach for exposing the standard actions, so let's wait a bit until we have a solution for that.

For comparison, here's what roborock a27 (so the same as you have, s7 maxv) using miot spec currently provides:

Detected model roborock.vacuum.a27                                                                                                                                                   
Returning data from cache file /home/tpr/.cache/python-miio/roborock.vacuum.a27.json                                                                                                 
Initialized: description='Robot Cleaner' urn=<URN urn:miot-spec-v2:device:vacuum:0000A006:roborock-a27:1 parent:None>                                                                
Created enum <enum 'Status'>                                                                                                                                                         
Created enum <enum 'Mode'>                                                                                                                                                           
vacuum:room-ids (Room IDs) reported no access information                                                                                                                            
generic-rpc:rpc-in (rpc-in) reported no access information                                                                                                                           
generic-rpc:rpc-out (rpc-out) reported no access information                                                                                                                         
roborock-vacuum:consumable-id (consumable-id) reported no access information                                                                                                         
roborock-vacuum:failed-reason (failed-reason) reported no access information                                                                                                         
roborock-vacuum:error-code (error-code) reported no access information                                                                                                               
roborock-vacuum:schedule-type (schedule-type) reported no access information                                                                                                         
Created 8 actions                                                                                                                                                                    
        ActionDescriptor(id='vacuum:start-sweep', name='Start Sweep', type=None, inputs=[], access=<Access.Execute: 5>)                                                              
        ActionDescriptor(id='vacuum:stop-sweeping', name='Stop Sweeping', type=None, inputs=[], access=<Access.Execute: 5>)                                                          
        ActionDescriptor(id='vacuum:start-mop', name='Start Mop', type=None, inputs=[], access=<Access.Execute: 5>)                                                                  
        ActionDescriptor(id='vacuum:start-sweep-mop', name='Start Sweep Mop', type=None, inputs=[], access=<Access.Execute: 5>)                                                      
        ActionDescriptor(id='vacuum:start-room-sweep', name='Start Room Sweep', type=None, inputs=[SensorDescriptor(id='vacuum:room-ids', name='Room IDs', type=<class 'str'>,       
access=<Access.Unknown: 1>, property='vacuum_room_ids', unit=None)], access=<Access.Execute: 5>)                                                                                     
        ActionDescriptor(id='battery:start-charge', name='Start Charge', type=None, inputs=[], access=<Access.Execute: 5>)                                                           
        ActionDescriptor(id='identify:identify', name='Identify', type=None, inputs=[], access=<Access.Execute: 5>)                                                                  
        ActionDescriptor(id='generic-rpc:generic-rpc-call', name='generic-rpc-call', type=None, inputs=[SensorDescriptor(id='generic-rpc:rpc-in', name='rpc-in', type=<class 'str'>, 
access=<Access.Unknown: 1>, property='generic_rpc_rpc_in', unit=None)], access=<Access.Execute: 5>)                                                                                  
Created 5 sensors                                                                                                                                                                    
        SensorDescriptor(id='vacuum:status', name='Status', type=<class 'int'>, access=<Access.Unknown: 1>, property='vacuum_status', unit=None)                                     
        SensorDescriptor(id='vacuum:fault', name='Device Fault', type=<class 'int'>, access=<Access.Unknown: 1>, property='vacuum_fault', unit=None)                                 
        SensorDescriptor(id='battery:battery-level', name='Battery Level', type=<class 'int'>, access=<Access.Unknown: 1>, property='battery_battery_level', unit=None)              
        SensorDescriptor(id='brush-cleaner:brush-life-level', name='Brush Life Level', type=<class 'int'>, access=<Access.Unknown: 1>, property='brush_cleaner_brush_life_level',    
unit=None)                                                                                                                                                                           
        SensorDescriptor(id='filter:filter-life-level', name='Filter Life Level', type=<class 'int'>, access=<Access.Unknown: 1>, property='filter_filter_life_level', unit=None)    
Created 1 settings                                                                                                                                                                   
        EnumSettingDescriptor(id='vacuum:mode', name='Mode', type=<class 'int'>, access=<Access.Unknown: 1>, property='vacuum_mode', unit='none', setting_type=<SettingType.Enum: 4>)

@starkillerOG
Copy link
Contributor Author

Alright, I understand, will close this for now then.

@rytilahti
Copy link
Owner

The linked PR adds an enum for "standard" vacuum descriptors, so I'm reopening this.

@rytilahti rytilahti reopened this Feb 10, 2023
@starkillerOG
Copy link
Contributor Author

@rytilahti this PR has been updated with the new vacuum identifiers, can you have a look?

@rytilahti rytilahti changed the title roborock use id instead of type Use standard identifiers for roborock Feb 11, 2023
Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you also add the id for the fan speed preset? I think that's the only missing one for full vacuum platform coverage.
Also, looks like homeassistant does not standardize the error message anymore, so that is not necessary but it does no harm so that could be cleaned up afterward.

@starkillerOG
Copy link
Contributor Author

@rytilahti Well I was a bit confused by the fan speed preset.
The fan_speed_presets in vacuum is a command, not a property.
That will need some further diging into and will probably break the current integration.
So I propose we leave that for a future PR.

@rytilahti
Copy link
Owner

Okay, let's leave it like this for now and clean it up later 👍 There is no need to worry about breaking current integrations, the new one (I have created a bare-bones custom component based on that old branch of mine) will be almost a complete rewrite anyway. I'll invite you to the repository as soon as I get it back into a working state :-)

@rytilahti rytilahti merged commit 2a33f41 into rytilahti:master Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants