feat: add specific device functionality #46
Conversation
|
I'm also adding a vacuummodelspecification which states which of the enum to use. I'll push that when I get home |
humbertogontijo
left a comment
humbertogontijo
left a comment
There was a problem hiding this comment.
Looks much better
| }, | ||
| ) | ||
| class RoborockStateCode(RoborockEnum): | ||
| starting = 1 |
There was a problem hiding this comment.
Shouldn't enum names be upper case? We could create a function to return it lower
There was a problem hiding this comment.
What would that function be? We would just call it on top of each time we need it lowercase?
I was thinking about translations of the entities, it my head it made more sense to be lower case but it could be upper case
There was a problem hiding this comment.
Let me test this when I have the chance. My first impression was that this is non standard. But if it helps with some scenarios I can work with it
There was a problem hiding this comment.
I wish I pushed my changes so that you could see it/ use it. But basically I have a function that builds a vacuum mode specification based on the model str that determines which e has to use
|
@humbertogontijo This still has more work - but let me know what you think conceptually whenever you have a chance - no rush |
| capability: Optional[Any] = None | ||
| category: Optional[Any] = None | ||
| schema: Optional[Any] = None | ||
| model_specification: ModelSpecification | None = None |
There was a problem hiding this comment.
Ops. I'm pretty sure this class is wrong. Might have been an issue on my end when coping and pasting all these entities
|
Overall it is looking fine |
|
Code is tested and working - you can see how I tested implementing it here: https://github.com/Lash-L/core/tree/roborock_device_specific/homeassistant/components/roborock |
| prefix=b"\x00\x00\x00\xa7", params={"data": [], "need_retry": 1} | ||
| ), | ||
| RoborockCommand.SET_CUSTOM_MODE: CommandInfo(prefix=b"\x00\x00\x00\x87", params=[108]), | ||
| RoborockCommand.SET_CUSTOM_MODE: CommandInfo(prefix=b"\x00\x00\x00w", params=[108]), |
There was a problem hiding this comment.
I had to change this to get it to work - was this change made on purpose?
There was a problem hiding this comment.
I sniffed commands and merged them into CommandInfoMap. It is strange that the app sends invalid prefixes haha
There was a problem hiding this comment.
Idk maybe there's some magic resting to prefixes we don't know yet haha.
Does the prefix appear to be wrong for you as well?
There was a problem hiding this comment.
Probably. I still don't know much about these 4 heading bytes
Yes, I had to apply your change for it to work.
|
After this gets merged, should we use dicts to parse the enum value? |
|
What do you mean by that? Like a dict from enum value to translation key? Why not use enum name as translation key |
|
Oh, I tried it just by logging the return of a command. But didn't verified that it was actually returning the enum for state and error_code |
|
I think this in its current form would work for that - this https://github.com/Lash-L/core/blob/34f175e59a722ce9a1c12bf0f64c2d079e446f4b/homeassistant/components/roborock/vacuum.py#L27 correctly maps, so I assume the errors would work as well. And whenever we need the string, we just call .name like I do here https://github.com/Lash-L/core/blob/34f175e59a722ce9a1c12bf0f64c2d079e446f4b/homeassistant/components/roborock/select.py#L44 This should be ready for a merge whenever you see fit. |
Lash-L
changed the title
|
Had to make a small change for translations to work |
|
You were able to get fan speed translations to work? How do? It wasn't translating for me when I tried |
|
I'm not there yet. Still making necessary changes |
2 participants
based this off of your single device work.
This is still very much WIP. but it is a basis and I thought I would publish it to see if you had any feedback