-
Notifications
You must be signed in to change notification settings - Fork 50
feat: improve dynamic clean modes #448
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?
Conversation
roborock/cli.py
Outdated
@@ -235,7 +235,7 @@ async def status(ctx, device_id): | |||
devices = home_data.devices + home_data.received_devices | |||
device = next(device for device in devices if device.duid == device_id) | |||
product_info: dict[str, HomeDataProduct] = {product.id: product for product in home_data.products} | |||
device_data = DeviceData(device, product_info[device.product_id].model) | |||
device_data = DeviceData(device, product_info[device.product_id].model, region=cache_data.user_data.region) |
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.
We are kind of in a weird situation.
We need the DeviceFeatures in order to create the valid options for Status - so it is stored in DeviceData. But we need DeviceData in order to set up the client - which we need to call get_init_status to get the DeviceFeatures.
Maybe the client just has a function to update it's "app_init_status" and then it updates it's status? Any thoughts?
I removed the more complicated changes that we can discuss in a follow up PR |
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.
It may be worth clarifying in clean_modes.py
what is internal vs part of the public api. One way you can do this is by setting __all__ = [ ... ]
to the list of public things or empty if this is totally private and internal.
my assumption is that file is moslty internal and the actual things we want callers to check will be in a trait when those are ready.
It is actually basically all external. You provide the device features, we will tell you which of the modes your device supports. |
There are some things i have to figure out (i.e. typing in some places)
And how to get locations to the trait
And how much of a breaking change is okay vs if i should try to make both possible at the same time