search

niceboygithub / AqaraGateway

Aqara Gateway/Hub integration for Home Assistant
523 stars 66 forks source link

Fix GatewayAction sensor not reset to default #284

Closed mikechouto closed 3 months ago

mikechouto commented 3 months ago

Thank you @niceboygithub

niceboygithub commented 3 months ago

@EricCorleone any option?

EricCorleone commented 3 months ago

As far as I know, the "unknown" state only appears when you reboot your HA and no event has been reported yet, and it should be the empty value None rather than the string "unknown".

mikechouto commented 3 months ago

I can change it to none instead of unknown but i'm sure that it not empty value in previous version of HA

EricCorleone commented 3 months ago

I'm not sure if you are talking about frontend display, because when the state is None, it will be displayed as "Unknown" on the frontend.

mikechouto commented 3 months ago

@niceboygithub @EricCorleone I've change the default value for the vibration sensor to none and repush the branch.

Thanks

EricCorleone commented 3 months ago

@niceboygithub @EricCorleone I've change the default value for the vibration sensor to none and repush the branch.

Thanks

Sorry I forgot to mention that using the empty string '' as the default value would be a better choice😂because it will be displayed completely blank on the frontend, which looks much better than "unknown" (which is none in the code). IMG_8888

mikechouto commented 3 months ago

@niceboygithub @EricCorleone I've change the default value for the vibration sensor to none and repush the branch. Thanks

Sorry I forgot to mention that using the empty string '' as the default value would be a better choice😂because it will be displayed completely blank on the frontend, which looks much better than "unknown" (which is none in the code). IMG_8888

Hi, sorry you might miss understood from my previous reply, when the state is none HA can correctly display it and it wouldn't show unknown please find the information in screenshot below. IMHO using a default state would be better than empty string as having a readable default state can bring more advantage when we write and maintain script/automation.

The default values might be able to be like none for sensors, off for switch... etc.

screenshot 2024-08-06 上午10 00 57
EricCorleone commented 3 months ago

The None I meant is not a string, but the empty value in python without quotation marks and the first letter is capital. It will be rendered as "unknown" on the frontend according to my last try. But I guess the string "unknown" also works according to your screenshot.

And the reason I prefer the empty string over "unknown" is simply because it looks better, but both are ok I guess (also I don't think I will use the empty/unknown state in automations).

mikechouto commented 3 months ago

Hey Eric yeah I've also tested the None in python which will be render as unknown on frontend. And I have some automation base on state changes so having a default value would let my automation be more readable LOL. I think we can wait for @niceboygithub to see what he would like the default value to be for long term usage, then i can modify. Thanks again for the detail explain.

Necroneco commented 3 months ago

Both "" and "none" are hardcoded. If I had to choose one, I'd choose "". I searched the code history and found that it was always "". If you change this default, you might break other people's automation.

mikechouto commented 3 months ago

@Necroneco Thanks for pointing it out I've modify the branch to just moved reset_state into GatewayBinarySensor class and the function will always be '' now.

Necroneco commented 3 months ago

But this is not much different from #280. Currently, only these two classes need this method.

mikechouto commented 3 months ago

@Necroneco I’m not sure if there are other devices that got effected as I don’d have a large variety to test against. And yeah merging both should be fine, I was just thinking to reduce duplicate code. ^_^