Advantage Air climate entity reports null for current_temperature

apat183 commented 1 year ago

The problem

Advantage Air integration doesn't set a current_temperature value. See example below.

For multi zone or other setups, I think the integration should either set the current_temperature as the main zone temp sensor or take an average of all the zones temps. The homebridge integration of this same integration here https://github.com/mitch7391/homebridge-cmd4-AdvantageAir takes the approach of setting the current_temperature as the main zone which I think is good approach.

Reason for this need is in homekit if there no current_temperature it doesn't show up on climate item, and it's just good to have current temp with climate. As with post here, https://community.home-assistant.io/t/climate-entity-advantage-air-set-current-temperature-based-on-template-sensor/592663. The mod seems to think there isn't a better way to do this other then report as part of integration issue. I'm guessing there is no way to override or set own current_temperature based on other sensors.

What version of Home Assistant Core has the issue?

core-2023.7.2

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Advantage Air

Link to integration documentation on our website

https://www.home-assistant.io/integrations/advantage_air

Diagnostics information

N/A

Example YAML snippet

N/A

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 1 year ago

Hey there @bre77, mind taking a look at this issue as it has been labeled with an integration (advantage_air) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of `advantage_air` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign advantage_air` Removes the current integration label and assignees on the issue, add the integration domain after the command.

_{^{(message by CodeOwnersMention)}}

advantage_air documentation advantage_air source _{^{(message by IssueLinks)}}

apat183 commented 1 year ago

@Bre77 just checking if you had any thoughts on above? Or if there was anyone else key on the core integration that can help?

Bre77 commented 1 year ago

Thanks for the prompt. I think I have a good way to solve this, but only for systems that have temperature sensors.

Are you saying a system like mine with no current temp anywhere doesn't work? That does sounds like an upstream issue.

apat183 commented 1 year ago

Thanks for the prompt. I think I have a good way to solve this, but only for systems that have temperature sensors.

Are you saying a system like mine with no current temp anywhere doesn't work? That does sounds like an upstream issue.

Yeah I would agree systems with current temperatures only. I dont expect the plugin to handle systems with no temperatures.

The simplest possible way for systems with zone controls would be to set the temperature as the main zone temperature sensor if it's available.

issue-triage-workflows[bot] commented 11 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

apat183 commented 11 months ago

Still not resolved or know if there is going to be something done?

Bre77 commented 11 months ago

I thought I did fix this, I at least wrote the code for it, maybe I just didn't tag the issue. Let me check.

Edit: turns out I had two unique issues that required a similar fix, which is why this one got lost of me.

I know how to fix this easily.

Bre77 commented 11 months ago

@apat183 does your select.*_myzone entity chance values while your in MyAuto or MyTemp mode? If so then I'll disable the current temperature of whatever is the selected zone rather than doing an average which could be misleading.

apat183 commented 11 months ago

@apat183 does your select.*_myzone entity chance values while your in MyAuto or MyTemp mode? If so then I'll disable the current temperature of whatever is the selected zone rather than doing an average which could be misleading.

I'm not fully sure but see screenshot. Happy to provide anything to help, I just didn't fully understand question.

Edit: Each zone has its own temp. Here is screenshot for the living one that is what has been set as the select.*_myzone

Bre77 commented 11 months ago

Assuming your system is on that helps, do you have the "MyTemp" or "MyAuto" mode enabled, or are you in "MyZone" mode (If you dont even have these options then your in MyZone).

apat183 commented 11 months ago

Assuming your system is on that helps, do you have the "MyTemp" or "MyAuto" mode enabled, or are you in "MyZone" mode (If you dont even have these options then your in MyZone).

Currently I have it set to myzone.

I do have other options, should I be using others? I read what they do and myzone seemed the best option for me.

Bre77 commented 11 months ago

Ok cool. What would be helpful for me, because I cannot test this myself, is if you change to one of the other modes for a few minutes and see if the MyZone value in home assistant changes to another zone.

I am going to assume it does for now and get the PR raised.

apat183 commented 11 months ago

Ok cool. What would be helpful for me, because I cannot test this myself, is if you change to one of the other modes for a few minutes and see if the MyZone value in home assistant changes to another zone.

I am going to assume it does for now and get the PR raised.

Yeah looks like it changed. I set to maintemp (which I think is just mytemp) and see screenshots. It doesn't have temp targets now and it changed myzone to a different zone (kitchen) by itself.

apat183 commented 11 months ago

Also just sharing the help page that explains the different options if that helps and confirms it's the same as other models. I know anywAiR is just a rebrand of advantageair

Bre77 commented 11 months ago

Perfect! The solution I have implemented is that it will show the temperature of the zone selected, be that manually or by MyAuto/MyTemp.

Thank you for doing that testing.

apat183 commented 11 months ago

Perfect! The solution I have implemented is that it will show the temperature of the zone selected, be that manually or by MyAuto/MyTemp.

Thank you for doing that testing.

No problem on the testing. Thank you for doing the change!! Much appreciated.

apat183 commented 11 months ago

Perfect! The solution I have implemented is that it will show the temperature of the zone selected, be that manually or by MyAuto/MyTemp.

Thank you for doing that testing.

Hi, is there anything I need to do to get integration showing the new temp? I reloaded the integration, but do I need to delete and add integration again?

See screenshots but still not showing.

jpbede commented 11 months ago

No, this will be shipped with the 2023.12 release.

apat183 commented 11 months ago

No, this will be shipped with the 2023.12 release.

Cool thanks, sorry saw PR was merged so just made assumption latest version included all latest PR's, is there an easy way to see which PR's are included in which versions?

jpbede commented 11 months ago

After the cut for the beta release (one week before the release), each PR that goes into the release is marked with a milestone. After the release, each PR that goes into a patch release gets marked with a milestone. Anything else goes into the next release. Hope this is clear :)

home-assistant / core