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

Sensor improvements #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

blueshiftlabs
Copy link

Hey there! I made some changes to how the EnergySmart bridge works. There's more details in each of the commit messages (and I can break this PR apart into several parallel requests if you'd prefer), but here's the highlights:

  • Added an option for debug logging of requests to the server
  • Added the option to expose the raw HTTP port, in case you're running an SSL-terminating reverse proxy elsewhere
  • Improvements to device and entity registration, to ensure that the overall water heater device in Home Assistant behaves correctly in environments where you have more than one water heater
    • Note that this is a breaking change - some of the sensors may change entity IDs and lose any previously applied customizations!
  • Add entity categories (controls/config/diagnostics) and device classes to entities
  • New entities for Wi-Fi signal strength and update rate

Happy to answer any questions about my approach here - thanks for maintaining this project!

- Expose port 8001 in config.yaml, so that users who are terminating SSL on another machine, because port 443 is in use on the Home Assistant host, can access the unproxied HTTP endpoint.
- Add a DEBUG flag that cranks up log level, including logging every request made to the server.
- Use jq to parse options in run.sh, rather than grep, and avoid using `eval`.
- Remove file-based logger that writes to a file that's inaccessible outside the Docker container. Rely on the Supervisor's logging of init's stdout instead.
- Convert all failure sensors to binary_sensors.
- Use MAC address and software information in device registry info.
- Use name of water heater as device name.
- Add entity_categories to diagnostic-type sensors, so they are categorized properly in the device view and are not automatically added to Lovelace dashboards.
- Add device classes to sensors as appropriate.
- Add Wi-Fi signal strength sensor.
This change adds a `number` entity that controls the update (polling)
rate for the EnergySmart dongle. It can be set anywhere between 30 and
300 seconds. This can be used to increase the responsiveness of sensor
updates and allow quicker changes in operating mode, in exchange for
increased network load.
@cmonteiro128
Copy link

@starsoccer Can this be merged in? I can't use this unless I have an option to use a port other than 443

"Hybrid" => "eco",
"Electric" => "electric",
"Vacation" => "off",
_ => "unknown",
Copy link
Owner

Choose a reason for hiding this comment

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

I dont think unknown is a supported state by HA. Ive not played around with HA much to know what will happen if we send an unsupported state which is why my prior code just fellback to off rather then something else. But open to change this if its safe to do so

default:
break;
}
string heater_mode = payload switch
Copy link
Owner

Choose a reason for hiding this comment

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

Since you seem way more familiar with C#, is there a way we can reuse this mapping both ways without having to repeat it here and in the other file?

{
Climate ret = new Climate
string unique_id_slug = (name ?? "__default__").ToLowerInvariant().Replace(' ', '_');
Copy link
Owner

Choose a reason for hiding this comment

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

What is with the default here? Sorry if this is a dumb C# question

@@ -10,6 +10,7 @@ public enum DeviceClass
{
heat,
problem,
moisture,
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure we should be using moisture for the leak detected? Since leak detected is just a boolean, and the HA docs say moisture is for a percentage, I am not sure this will map or display properly in the frontend

@starsoccer
Copy link
Owner

starsoccer commented Apr 8, 2024

@starsoccer Can this be merged in? I can't use this unless I have an option to use a port other than 443

@cmonteiro128, I just left comments on the PR based on my limited knowledge of C# and my understanding of HA and the mqtt specs.

@blueshiftlabs Just want to apologize for the delay in reviewing this. I appreciate you taking the time to make a PR. For some reason I must have missed the github email for this PR.

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.

3 participants