RobertD502 / hass-flair-helper

Flair API helper used by home-assistant-flair component
MIT License
2 stars 2 forks source link

HvacUnit.refresh failing on Flair initialization #3

Closed smcpeck closed 2 years ago

smcpeck commented 2 years ago

https://github.com/RobertD502/hass-flair-helper/blob/53807f9f5d5ada1fa1cfbab2b056a991614b764b/flair/hvac_units/hvac_unit.py#L21

I don't have a temperature-scale key in my constraints portion of the response. It looks like the rest of the references to constraints are as one would expect.

My dictionary keys are as follows:

constraints: ['ON', 'OFF']
ON: ['DRY', 'FAN', 'AUTO', 'COOL', 'HEAT']
OFF: ['COOL']

FWIW, I don't see temperature-scale in my response at all... 🤷‍♂️

Let me know if I can provide additional details to help.

EDIT: To be completely informative, this is when being consumed by the dev branch in the home-assistant-flair repo.

RobertD502 commented 2 years ago

Now that is REALLY odd because Flair's own documentation states that the constraints field lists the temperature unit and that temperature values sent to the HVAC should match the temperature unit that is seen in the constraints field. My constraints do list a temperature-scale key. Would you mind doing a curl to the structure that the mini split is associated with and let me know if there is a temperature-scale key within the attributes of the structure. If there is I can add a check to the code to use the temperature-scale key from the constraints if it exists and if not use the temperature-scale key from the structure that the mini split is associated with.

image

Edit: Would you also mind uploading a screenshot of the room that has the mini split in it in the Flair app? Just want to see what the temperature scale shows there.

smcpeck commented 2 years ago

I'll poke around a bit more at what the API is returning for my devices and get back to you.

Here is what the Flair Android app shows for the room:

RobertD502 commented 2 years ago

You sure like it toasty in that room. I think using the scale returned by the structure will take care of the majority of people whose constraints field doesn't have the temp scale listed but it could be a problem for some. If someone has a mini split that is controlled in Celsius (but they don't have a temp scale constraint) and their Flair structure is set to Fahrenheit, then sending Fahrenheit values won't work on the mini split. I may be overthinking this and this scenario is highly unlikely.

smcpeck commented 2 years ago

This mini-split is in Central America - so it is naturally toasty. 😆

That said, I did (at one point) have the Puck showing Celsius while I had my app showing Fahrenheit -- can't recall how I accomplished that, but it took some finagling... maybe I broke/confused something, heh.

RobertD502 commented 2 years ago

Does the mini-split, located in Central America, use Fahrenheit on its remote or celsius?

smcpeck commented 2 years ago

I'd have to double check... I think the standard remote and the Puck are both C and my app is F. But that may not be true. I'll need some time to get someone to check on that for me.

smcpeck commented 2 years ago

UPDATE: Two Flair Pucks down there, neither has temperature-scale on the constraints attribute.

A GET /structures call returns an object for the property that has a temperature-scale attributes which reads 'temperature-scale': 'F'. Maybe that's what should be consumed instead of looking on this constraints attribute for each hvac_unit? I do believe that is set at a parent level instead of on each Puck. So I can't have one Puck in F and the other in C. They both inherit that setting from the Structure.

RobertD502 commented 2 years ago

It might also be set based on the IR code that the puck downloaded. It is possible that they accidentally omitted that key for your IR codes. In the meantime I have implemented a safety check to use the key in constraints if it exists and if not use the key found in the structure endpoint. The changes have been made. Restart Home Assistant and let me know if that fixed it for you.

Edit: I also contacted Flair's head of hardware engineering to get an explanation about why your constraints don't have that key. Will be interesting to find out the reason.

RobertD502 commented 2 years ago

@smcpeck just received a response. Apparently the constraints field should have temperature-scale for all minisplits. He is asking me to provide the email address that is associated with your Flair account so he can look into it. Mind letting me know what e-mail address to send to him?

smcpeck commented 2 years ago

My e-mail is my github username at google's popular e-mail service.

Another interesting side note is that my structure is in "F" and I can clearly see that both mini-splits are in "C" based on the supported temps.

A funny sneak peek: image

RobertD502 commented 2 years ago

Could you do a request to the hvac unit endpoint and let me know what value is listed under the temperature attribute?

Edit: remember how I said I was overthinking the scenario of someone having their structure in F, but their mini split is in C even though they don't have a temperature-scale constraint? Looks like your case is exactly what I was worried about.

smcpeck commented 2 years ago

For the two mini-splits, I see ["attributes"]["temperature"] gives me values of 23 and 29. Definitely C.

My use case is that this is a rental unit I own in Costa Rica. So I want things in C for most of my guests, but I want things in F for myself.

Heck -- I could even have an automation down the road that changes what guests see in the apartment depending on some condition (maybe I look at the country code in their phone number?)... so yeah, I guess I'm the perfect use case to cause issues. 😁

A peculiar thing I've noticed on my structure object:

    'set-point-temperature-c': 25.57,
    'temp-away-max-c': 25.0,
    'temperature-scale': 'F',

Very weird that the structure scale shows F, but all the other temperature-based settings are very clearly in C. I suspect that it is because of my use case, but I still feel like the Flair API is returning odd/confusing results here.

RobertD502 commented 2 years ago

Yeah your Mini Split set temp is definitely returning temps in C. Regarding the structure stuff, all of the temps returned by the API are in C which is why they added -c to the key naming. One deviation from this is the mini split set-point ("temperature" key in the hvac attributes) which returns the temp based on the constraints field. In my case the temp is returned in F, but all other endpoints in the API show C values. The temperature scale for your structure just refers to what units you are using in the app.

RobertD502 commented 2 years ago

@smcpeck update from Flair:

`I dug a bit more, it looks like there are a lot of minisplit codesets from 2017 that are missing the temperature-scale key.

I've fixed the issue manually for the two minisplits this user reported. We're going to go through and clean up the rest with a script this week.`

So it looks like omission within the IR codes is the culprit.

RobertD502 commented 2 years ago

I have added a C vs F check based on temp range that should take care of this issue until Flair fixes their IR codesets.