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 device_class for sensors published to Home Assistant #471

Merged
merged 4 commits into from
Feb 25, 2025

Conversation

paulhomes
Copy link
Contributor

Adds the device_class attribute to sensors published to Home Assistant so they can be charted alongside other entities of a similar type - see issue #470

@davidusb-geek
Copy link
Owner

@paulhomes Just pushed a fix for unit testing

@paulhomes
Copy link
Contributor Author

This branch has been working for me, and I think is potentially ready to merge, but there are a few places where I have specifically not added any handling for device_class as they are not actions that I have used as yet and am unsure if device_class is needed or wanted in those locations. More specifically:

  1. In command_line.py I haven't added a model_predict_device_class in the forecast_model_predict() function around line 801 nor a mlr_predict_device_class in the regressor_model_predict function around line 988 and likewise added no equivalent handling of a potential model_predict_device_class in the utils.py treat_runtimeparams() function around line 659 nor mlr_predict_device_class around line 680
  2. In the retrieve_hass.py post_data() function I have not added device_class in the handling of type_var == "optim_status" around line 602 and type_var == "mlregressor" around line 610

In reviewing this PR for potential merging please could you take a quick look at those and advise on whether you think device_class is warranted in those places (or should be left for another time).

Thanks

@paulhomes paulhomes marked this pull request as ready for review February 25, 2025 11:52
@davidusb-geek
Copy link
Owner

Thanks for pointing out all those cases, yes they need to be considered.
Also there are a bunch of additions to utils.py for this. It's almost done, I will push and probably merge when finsihed

@davidusb-geek davidusb-geek merged commit a37bc3e into davidusb-geek:master Feb 25, 2025
10 of 16 checks passed
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.

2 participants