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

Added file for new device: Eurom Sani towel radiator #1810

Merged
merged 16 commits into from Apr 27, 2024

Conversation

jan-gerard
Copy link
Contributor

Hi,
I added the device "Eurom Sani-Bathroom-Radiator Towel Wifi heater". I tested it with the radiator.
One thing that deviates from the manual, is that I created a non-existing dummy DP id for the current temperature, while maintaining the availability of the two sensor values: room temperature and surface temperature. The heater can control both, depending on the mode of operation, whether it controls the room or the heater itself.
Furthermore I wanted to specify an icon for the new operating mode 'anti-frost' but I did not manage to find out how to do that, using the icons.json file. If you have any suggestions?
Jan-Gerard

Added device "Eurom Sani-Bathroom-Radiator Towel Wifi heater"
@jan-gerard
Copy link
Contributor Author

jan-gerard commented Apr 9, 2024

Fixed: "Ai, I notice now that setting the eco preset-mode does not work from Home Assistant. All other presets do work. And retrieving the preset-mode from the device works as well. The difficulty is that triggers are two binary switches that are mutually exclusive. How can I combine two binary entities into one (string) preset-mode, providing both getting and setting? I need nested conditions?" by changing String into Boolean for the Preset-mode. Now everything is working, except for a nice icon for the anti-frost mode.

@jan-gerard
Copy link
Contributor Author

I fixed the two errors in the code. Now it should pass the tests.

@jan-gerard
Copy link
Contributor Author

There is one annoying thing: Switching preset modes from Anti-frost to Eco is only possible when switching first to preset None. Because the preset-mode is a combination of two boolean entities, and the condition statement is used, then for one of the states only one of the booleans is relevant, and the other is not switched when switching mode. I don't see a method to solve this. It's not blocking though.

@make-all
Copy link
Owner

make-all commented Apr 11, 2024

You can mark entries in a conditional mapping as hidden: true to avoid selecting that one - useful when you want selecting eco to always set anti-frost off, but you still want to recognise it as eco if both anti-frost and eco are on (or vice versa). In that case, marking the "both true" case as hidden will prevent that from being used when setting.

@jan-gerard
Copy link
Contributor Author

jan-gerard commented Apr 11, 2024

You can mark entries in a conditional mapping as hidden: true to avoid selecting that one - useful when you want selecting eco to always set anti-frost off, but you still want to recognise it as eco if both anti-frost and eco are on (or vice versa). In that case, marking the "both true" case as hidden will prevent that from being used when setting.

I tried that, but it doesn't work. Probably, the values are set the right way, but first both mode should be set to false, before the other can go go true, in the heater firmware. When I read the diagnostic logging, the 102:false-->true and 103:true-->false transitions are sent at the same time. Then the heater does not accept them, and ends up setting 103 to false but fails to set 102 to true.
There seems to be no other resolution than the send them one after another, either using an automation in home assistant, or by making two individual boolean switches and not using presets.

fixed heating state by removing the quotes from "off"
@jan-gerard
Copy link
Contributor Author

I think I can live with the limited preset switching flow. Who wants to switch from Eco to Anti-frost at once. So I think the code is now optimal. And the heating state does not show the drying state anymore: heating means heater is on, also when used for drying.

@make-all
Copy link
Owner

If the device does not support setting two dps together, then I suggest that you simplify the config and do not try to combine the eco and anti-frost together. Make anti_frost a separate switch, and just leave "eco" and "comfort" as presets.

"off" requires the quotes in hvac_mode and hvac_action, as it is defined in HA as the string "off", not the boolean value False, which is what yaml converts unquoted off to.

hvac_action should report drying if the hvac_mode is dry. If that breaks the chart display, then that is a bug in the frontend, the integration should not be crippled to workaround frontend bugs.

Fixed incorrect hvac_action value off (changed to "off").
This version still has the Eco and Anti-frost modes.
@jan-gerard
Copy link
Contributor Author

off changed to "off", and hvac_action Dry is restored. Only I'm still struggling to get an Eco mode switch in the HA front-end. In my opinion, the Anti-frost mode is more relevant as preset, than the Eco mode (which is in fact only a 16 C room-temperature setpoint).

Changed presets to 'none' and 'anti-frost'. Eco mode is a separate switch (but not visible in HA climate element).
@make-all
Copy link
Owner

I just realised I have some unsent comments, so at least one of them is probably obsolete now.

min: 40
max: 70
- id: 333
# dummy variable for current temperature of
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use one of the actual dps instead of making one up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the point, I need to create an entity that doesn't exist, since the others (heater_temperature:105 and room_temperature:3) are already used. And the "current_temperature" that is used depends on the operation mode:2. I could use one of the above as the base entity (let's say heater_temperature), but then that one behaves rather strange in the logging. So the current_temperature entity is the actual temperature as used by the internal control loop, corresponding to the mode of operation.
If there is another option to create an entity without a dps id, I'd like to use that, but for now, this seems to be the only workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is it possible to use the same dp id in both the primary and secondary entities? That would allow the actual values to be used (heater surface temperature and room temperature) as individual sensors (nice for making graphs) and use either of the two as the current temperature in the climate/heater device. That would be most elegant and correct I think.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is possible to reuse the same dp in another entity, or even in the same entity with a different name. Making secondary sensor entities for both temperatures would make them easier to work with than as non-standard attributes on the climate entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a change accordingly.

# todo; icon
value: "Anti-frost"
- dps_val: true
value: "eco"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The true case should also have constraint and conditions to ensure the two values are always set together.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The true case should also have constraint and conditions to ensure the two values are always set together.

That appears to be a bit of a problem. When one of the modes is on, to switch to the other, the first should first be switched off. So Eco to Anti-frost switching should first switch Eco off, then in a next all switch Anti-frost on.

fixed: removed boolean value mapping, and added name:switch
The separate temperatures (room and heater surface) are now secondary entities, and the current temperature no longer uses a dummy dp id.
Copy link
Owner

@make-all make-all left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commented out portions can just be removed if it is working. This will resolve the lint warnings about yamllint's fussy indentation preferences

type: boolean
mapping:
- dps_val: false
value: "none"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appropriate standard presets would be home and away

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appropriate standard presets would be home and away

That appears to be a bit of a problem. When one of the modes is on, to switch to the other, the first should first be switched off. So Eco to Anti-frost switching should first switch Eco off, then in a next all switch Anti-frost on. We can make both the switches boolean secondary entities, but they should still be mutually exclusive, and to function properly, the switching should be a 2-step process.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the device does not accept these being switched together, then we don't really have a way to enforce that. Perhaps you can make some buttons in the UI that are tied to automations that do the correct sequence of events. In that case you may want to move both out to external switches which can be hidden from direct use in the UI, to avoid someone accidentally using the climate presets while the eco switch is not in the correct state.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had hoped there would be another way, to implement all functions of the heater, elegantly in the standard Climate entity format, while using this integration. Apparently the Tuya app also sets those in succession. I guess there are more Tuya devices out there that suffer from the same behavior.
But I understand that it is not straightforward implementation, while maintaining the elegant condition scheme that implicitly sets multiple states according to the yaml file of the device. Maybe adding some statements that define an order of entities to be set, which then determines whether values/switches can be set at once, or need successive calls to the API? Or that define 'invalid combinations', such that the integration knows that two contradicting states are not valid, then queues the states that are not allowed in combinations with others, and iterates until all combinations are valid and set. So if 'invalid: eco=on & away=on' is set, then switching from 'eco=on&away=off' will first switch to the valid 'eco=off&away=off', notices that it's not ready since away should be on in the end, and makes a second call to 'eco=off&away=on'. No defined sequence needed, only defining invalid combinations.

Updated name and id.
Changed preset mode to home and away.
Removed obsolete comments.
Added icons for secondary entities.
@jan-gerard
Copy link
Contributor Author

For me the integration works fine now. I can control the things I need, and include them in the Climate card in HA.

- move all branding to products section
- remove unneeded quotes from strings
- remove old commented line that was giving lint warning

PR make-all#1810
@make-all make-all merged commit 71a4d98 into make-all:main Apr 27, 2024
4 checks passed
make-all added a commit that referenced this pull request Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants