search

rianadon / opensprinkler-card

Home Assistant card for collecting OpenSprinkler status
Other
66 stars 8 forks source link

Safer check for control types #5

Closed vinteo closed 3 years ago

vinteo commented 3 years ago

I wanted to fix the controller check but ended up doing some refactor in the end. Hope it is ok.

rianadon commented 3 years ago

Great thinking! While you're at at it, these lines for finding the sensors to put int the popup could also benefit from the new checks:

https://github.com/rianadon/opensprinkler-card/blob/7c12b99a2f49a5fb5fb58350280f4a1934288e4e/src/opensprinkler-more-info-dialog.ts#L103-L109

At least the enabled sensor will, I think, fail if the device is named something different?

Also if you are in the mood for refactoring, I wonder if, now that the isStation and isProgram checks are more robust, we could replace getControlType and the ControlType enum with calls to isStation, isProgram, etc.

vinteo commented 3 years ago

Have cleaned up the rendering of the dialog entities a little.

rianadon commented 3 years ago

Any ideas on how to keep the order of the sensors? I liked having sensor1/sensor2 at the bottom of the list and rain delay active / rain delay stop time grouped together.

vinteo commented 3 years ago

I think the only way right now is to group and sort by the entity_id

vinteo commented 3 years ago

This some sorting

image

rianadon commented 3 years ago

I just cleaned this up a little by removing ControlType. I like the order now! Thanks so much for taking my suggestions and all the work!