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

Monitor as switch, linked command with sensor, battery fix #37

Merged
merged 10 commits into from
Jan 26, 2023

Conversation

infeeeee
Copy link
Collaborator

Retrying #29

@richibrics
Copy link
Owner

Hi @infeeeee, your solution was in the right way, correct to edit only the configurations for hass and not also the way states are sent.
I preferred using a link between command and sensor to explicit the fact that the command has a state in the entire system.

@infeeeee
Copy link
Collaborator Author

The state_topic and the command_topic have to be the same on a linked command. If they are different, than the switch "blinking" on HA, as when you change the switch the state doesn't change automatically, so the flow is something like this:

  1. You flip the switch to ON on HA, it changes its state there to ON
  2. IoTuring runs the command
  3. HA checks the state of the switch and it's still OFF, so the switch changes back to OFF
  4. IoTuring updates the state to ON on the next sensor update cycle
  5. HA changes the state to ON

So on HA it looks like ON-OFF-ON, not nice.

The optimistic flag should solve this, but it changes the switch to a double command on the UI, it's not nice :(

https://www.home-assistant.io/integrations/switch.mqtt/#optimistic

So I recommend using the same KEY and topic for both the sensor and the command. Is there any bad side effect for this?
That was the reason I linked them by key, not by another parameter.

Or the callback should also update the sensor automatically, but simply using the same key would be easier.

@infeeeee
Copy link
Collaborator Author

I just added a really simple FileSwitch entity, it's easier to debug linked commands with this, you can see this "blinking" behavior, I'm speaking about, why I recommend having the same topic for state and command topic.

@richibrics
Copy link
Owner

Okay, got the problem. The reason I wanted to use a different key is because if the sensor data is sent as sensor (for non homeassistant warehouse), the key of the sensor may not define what the sensor value is, if equal to command key (maybe not in this case for monitor)

@richibrics
Copy link
Owner

So currently the solution I found is to send state value to command topic if it's linked to a command in the home assistant warehouse and do not use its key for the topic

@richibrics
Copy link
Owner

richibrics commented Jan 15, 2023

@infeeeee but if we send sensor state on command state we continue to call the command callback; I don't think it is a correct behavior.

@richibrics
Copy link
Owner

No solution found, I think we have to set different topics for sensor and command :(

@infeeeee
Copy link
Collaborator Author

If you want to have different topics, other option would be to update the sensor after a callback. Not with the other sensors, but an extra update.

Also still don't get why is it a problem, if they have the same topic. I get it's a problem, if the key is the same, but another solution could be in this case NOT use the key as the topic, but something else. Both MQTT and HA should let you use the same topic, it's only the KEY is the problem.

That's 2 possible solutions

@richibrics
Copy link
Owner

The problem is not with the key, but with the message that the sensor sends to home assistant.
Sending the message to the same topic the command is subscribed to, will trigger the command.
So the command are now triggered by

  • HomeAssistant when triggeredd by the user
  • IOTuring itself when sends sensor data (every 10 seconds).

And the command triggered every 10 seconds is not a great thing in my opinion. What do you think ?

@infeeeee
Copy link
Collaborator Author

I see. Maybe a check in the callback, to only run if the command state is not the same as the current state? Do nothing if it's the same.

Or my other solution with different topics: send one out of loop update to the sensor, instantly, from the callback. I can't see a downside of this way. Maybe miss the next sensor update, in this case, so they won't run at the same time accidentally.

@richibrics
Copy link
Owner

With this commit, when a command is triggered from HA warehouse, if the command is a switch, also send the just received state to the topic of the sensor relative to the switch

@richibrics
Copy link
Owner

So with this edit we can use different topics for sensor and command in my opinion. What do you think ?

Now I need to separate the two topics (sensor and command) which were re-joined

@infeeeee
Copy link
Collaborator Author

I will take a look

@richibrics
Copy link
Owner

With these changes it seems solid

@infeeeee
Copy link
Collaborator Author

I will check tomorrow, or later today, don't pull it yet

@richibrics
Copy link
Owner

Yes I would have waited for you anyway :)

@infeeeee
Copy link
Collaborator Author

So I added some small fixes:

  • For the FileSwitch extra attribute, it makes more sense to send the path as an extra data :)
  • data_type == 'switch' != entityData.SupportsState():
    • You can have a switch without state (Monitor command works this way on windows)
    • You can set a custom datatype in the yaml, so it's not always a switch.

I tried to solve this, adding a new local variable entitycommand_supports_state was easier, than adding a lot of multi level ifs

If it's ok for you, you can pull it, I don't want to add any more changes. 🚀

@richibrics
Copy link
Owner

Nice ! Yes I think it's time 🚀

@richibrics richibrics merged commit f9cc9ac into richibrics:main Jan 26, 2023
@infeeeee infeeeee deleted the monitor-switch branch January 26, 2023 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants