search

audiconnect / audi_connect_ha

Adds an audi connect integration to home assistant
MIT License
224 stars 98 forks source link

fix: False bools are not passed #398

Closed coreywillwhat closed 4 months ago

coreywillwhat commented 4 months ago

If the API is returning a False value for a sensor, the previous logic for _tryAppendFieldWithTs would not append. We need to specifically check val is not None and append the value.

This PR should fix that. This new logic is already used in _tryAppendStateWithTs, it just never made it into _tryAppendFieldWithTs for some reason.

See #383 for more details.

coreywillwhat commented 4 months ago

I've tested this with no ill effects on my end.

coreywillwhat commented 4 months ago

@neil-bh @wigster if either of you can test this PR, please report back on the results. I can't test as my car doesn't report this value.

wigster commented 4 months ago

As far as I can tell, this is fine for me -- no oil sensor at all, otherwise things work. Is that what is supposed to happen?

Q5 Hybrid 2019.

wigster commented 4 months ago

Having said this -- i have just looked at myaudi.com (as opposed to the app). There in my vehicle state I do have an oil level sensor reporting (as "Not OK").

neil-bh commented 4 months ago

Excuse my ignorance, but how do I test this PR specifically? I assume I need to manually pull a specific commit or something like that? Currently I just have HA using HACS to pull the latest release.

coreywillwhat commented 4 months ago

The easiest method in my opinion is to Install studio code server add on for HA, open it, navigate to the file where the changes are (custom components > audiconnect > models.py) and modify the files/lines. You can either replace the exact lines or just copy in the entire file.

Then restart HA.

You can also modify the files directly without studio code server if you have access to those, it would be the same process just from a file explorer and restart HA after.

You can find the changes under "files changed" at the top of this PR. It will show the lines to remove in red and lines to add in green.

coreywillwhat commented 4 months ago

Having said this -- i have just looked at myaudi.com (as opposed to the app). There in my vehicle state I do have an oil level sensor reporting (as "Not OK").

According to your log and value of False and app saying not okay, you should have a binary sensor. You tried modifying the files and restarting HA and still no binary sensor after a cloud update?

coreywillwhat commented 4 months ago

Also if neither of you are comfortable we can merge this and you can try once it's in the next release and let us know if it fixes it or not.

neil-bh commented 4 months ago

The easiest method in my opinion is to Install studio code server add on for HA, open it, navigate to the file where the changes are (custom components > audiconnect > models.py) and modify the files/lines. You can either replace the exact lines or just copy in the entire file.

Then restart HA.

You can also modify the files directly without studio code server if you have access to those, it would be the same process just from a file explorer and restart HA after.

You can find the changes under "files changed" at the top of this PR. It will show the lines to remove in red and lines to add in green.

This was the method I used, I thought it was rather unconventional and not a developers approach, but nevertheless, I replaced the entire file rather than add/remove lines based on the - and + markup accordingly, because I'm lazy.

And law and behold, a restart of HA now shows that I have a binary_sensor.audi_a6_limousine_oil_level_binary entity that is reporting the value "OK".

So with that said, looks like it's working as per your intentions. I'm giving a green light to merge.

coreywillwhat commented 4 months ago

The easiest method in my opinion is to Install studio code server add on for HA, open it, navigate to the file where the changes are (custom components > audiconnect > models.py) and modify the files/lines. You can either replace the exact lines or just copy in the entire file.

Then restart HA.

You can also modify the files directly without studio code server if you have access to those, it would be the same process just from a file explorer and restart HA after.

You can find the changes under "files changed" at the top of this PR. It will show the lines to remove in red and lines to add in green.

This was the method I used, I thought it was rather unconventional and not a developers approach, but nevertheless, I replaced the entire file rather than add/remove lines based on the - and + markup accordingly, because I'm lazy.

And law and behold, a restart of HA now shows that I have a binary_sensor.audi_a6_limousine_oil_level_binary entity that is reporting the value "OK".

So with that said, looks like it's working as per your intentions. I'm giving a green light to merge.

Thanks for testing! The value of OK is expected? Same as in your Audi app? I'm losing track of who had what values haha

neil-bh commented 4 months ago

The easiest method in my opinion is to Install studio code server add on for HA, open it, navigate to the file where the changes are (custom components > audiconnect > models.py) and modify the files/lines. You can either replace the exact lines or just copy in the entire file. Then restart HA. You can also modify the files directly without studio code server if you have access to those, it would be the same process just from a file explorer and restart HA after. You can find the changes under "files changed" at the top of this PR. It will show the lines to remove in red and lines to add in green.

This was the method I used, I thought it was rather unconventional and not a developers approach, but nevertheless, I replaced the entire file rather than add/remove lines based on the - and + markup accordingly, because I'm lazy. And law and behold, a restart of HA now shows that I have a binary_sensor.audi_a6_limousine_oil_level_binary entity that is reporting the value "OK". So with that said, looks like it's working as per your intentions. I'm giving a green light to merge.

Thanks for testing! The value of OK is expected? Same as in your Audi app? I'm losing track of who had what values haha

In my case, yes. The expected value should be OK. My Audi app shows OK too.

Kolbi commented 4 months ago

@coreywillwhat PR is still in Draft? I'm gonna merge it with our okay.

coreywillwhat commented 4 months ago

@coreywillwhat PR is still in Draft? I'm gonna merge it with our okay.

Yeah you can merge. Baby coming today so I'll be hit or miss for a few weeks

Kolbi commented 4 months ago

I wish you all the best!

wigster commented 4 months ago

I confirm that I have the binary back after the upgrade 1.6.3