core icon indicating copy to clipboard operation
core copied to clipboard

Fix Tuya Fahrenheit device temperature readings

Open DellanX opened this issue 1 year ago • 7 comments

Attempt to re-implement preferred unit's effect on climate. I think Tuya recently changed how the device temperature is reported, as I now see a Unit property under TEMP_CURRENT that reports Fahrenheit, but this code needs modification to read that.

NOTE: These changes aren't validated. I am busy with getting married and other things, so I don't have time to setup a development environment to validate these changes. I've done my best to ensure that code changes are stable, and easy to read.

Proposed change

Modifying the code to better handle the implicit type of the Tuya temperature readings. This is done by leveraging the existing preferred unit variables.

Type of change

  • [ ] Dependency upgrade
  • [X] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [ ] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #124119
  • This PR is related to issue: #124119
  • Link to documentation pull request: NA

Checklist

  • [ ] The code change is tested and works locally.
  • [ ] Local tests pass. Your PR cannot be merged unless tests pass
  • [X] There is no commented out code in this PR.
  • [X] I have followed the development checklist
  • [X] I have followed the perfect PR recommendations
  • [ ] The code has been formatted using Ruff (ruff format homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed: NA, can I delete this section?

If the code communicates with devices, web services, or third-party tools: NA, can I delete this section?

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

DellanX avatar Aug 17 '24 14:08 DellanX

Hey there @tuya, @zlinoliver, @frenck, mind taking a look at this pull request as it has been labeled with an integration (tuya) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of tuya can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign tuya Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

home-assistant[bot] avatar Aug 17 '24 14:08 home-assistant[bot]

@frenck this looks like a pretty easy commit to accept...am I missing something?

poppainu avatar Oct 04 '24 17:10 poppainu

@frenck this looks like a pretty easy commit to accept...am I missing something?

Not sure why the ping, it is in draft and failing CI, it cannot be accepted atm.

../Frenck

frenck avatar Oct 04 '24 17:10 frenck

Correct, it's not ready yet. Specifically, it needs someone to validate it, and to get CI happy.

I may have time to help late January or early February.

DellanX avatar Oct 04 '24 17:10 DellanX

From what I see, the linter is failing on misspellings of "Celsius".


From: Dyllan Macias @.> Sent: Friday, October 4, 2024 10:52 AM To: home-assistant/core @.> Cc: poppainu @.>; Comment @.> Subject: Re: [home-assistant/core] Fix Tuya Fahrenheit device temperature readings (PR #124121)

Correct, it's not ready yet. Specifically, it needs someone to validate it, and to get CI happy.

I may have time to help late January or early February.

— Reply to this email directly, view it on GitHubhttps://github.com/home-assistant/core/pull/124121#issuecomment-2394275811, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADNQKB7XFTEO7NU7VNMX6M3ZZ3IWJAVCNFSM6AAAAABMVPG6VGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJUGI3TKOBRGE. You are receiving this because you commented.Message ID: @.***>

poppainu avatar Oct 04 '24 17:10 poppainu

Though. Looks like CI fails due to spellchecks. That's probably am easy fix

DellanX avatar Oct 04 '24 17:10 DellanX

Though. Looks like CI fails due to spellchecks. That's probably am easy fix

DellanX avatar Oct 04 '24 17:10 DellanX

Okay, I have ran the code on my side and am able to validate that it is working correctly. I also fixed the typos.

  • I am very busy planning wedding stuff, so I don't have time to add tests.
  • ~~I do not know how to resolve "Commit(s) are missing a linked GitHub user.", if I did, I'd link myself~~ I see, I accidentally used the wrong account to commit my changes. For pushed to fix

DellanX avatar Oct 20 '24 17:10 DellanX

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

github-actions[bot] avatar Jan 11 '25 16:01 github-actions[bot]