Skip to content

Fix ValueError for non-numeric value#166300

Open
LG-ThinQ-Integration wants to merge 9 commits intohome-assistant:devfrom
LG-ThinQ-Integration:pr_sensor_26.03
Open

Fix ValueError for non-numeric value#166300
LG-ThinQ-Integration wants to merge 9 commits intohome-assistant:devfrom
LG-ThinQ-Integration:pr_sensor_26.03

Conversation

@LG-ThinQ-Integration
Copy link
Copy Markdown
Contributor

@LG-ThinQ-Integration LG-ThinQ-Integration commented Mar 24, 2026

Proposed change

  1. Problem for KIMCHI_REFRIGERATOR's temperature sensor:
    The value of a refrigerator's temperature sensor is both a number and a string. Previously, there was no problem even if the value was a string and had a unit, but currently, a ValueError occurs, preventing the entity from being activated.
image
  1. Error:
    ValueError: Sensor sensor.2reb1krs11_h_middle_temperature has device class 'None', state class 'None' unit '°C' and suggested precision 'None' thus indicating it has a numeric value; however, it has the non-numeric value: 'kimchi' (<class 'str'>)

  2. Fix:
    Modified to set 'unit_of_measurement' only when the data.value is a number.

image

Type of change

  • Dependency upgrade
  • 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 #
  • This PR is related to issue:
  • Link to documentation pull request:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • 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.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • 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 diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

To help with the load of incoming pull requests:

Copilot AI review requested due to automatic review settings March 24, 2026 03:17
@home-assistant home-assistant Bot added bugfix cla-signed integration: lg_thinq small-pr PRs with less than 30 lines. Top 200 Integration is ranked within the top 200 by usage by-code-owner Quality Scale: No score labels Mar 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a Home Assistant sensor validation ValueError in the lg_thinq integration by avoiding setting a unit of measurement when a ThinQ property reports a non-numeric string value.

Changes:

  • Gate assignment of native_unit_of_measurement behind a numeric-type check for data.value.
  • Extend the debug log message to include the runtime type of data.value.

Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
@LG-ThinQ-Integration LG-ThinQ-Integration changed the title Fix ValueError for non-numeric value: 'storage' Fix ValueError for non-numeric value Mar 24, 2026
@LG-ThinQ-Integration LG-ThinQ-Integration marked this pull request as ready for review March 24, 2026 03:25
Copilot AI review requested due to automatic review settings March 24, 2026 05:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh hi there @LG-ThinQ-Integration 👋

Thanks for the pull request. However, Copilot did catch some issues I agree with.

Additionally, this PR is missing tests. Please add a regression test that covers a sensor whose API value becomes a non-numeric string while a unit would normally be set, verifying that native_unit_of_measurement is cleared and no ValueError is raised.

../Frenck

                       

Blogging my personal ramblings at frenck.dev

@home-assistant
Copy link
Copy Markdown

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant Bot marked this pull request as draft March 24, 2026 06:42
@LG-ThinQ-Integration LG-ThinQ-Integration marked this pull request as ready for review March 25, 2026 01:35
Copilot AI review requested due to automatic review settings March 25, 2026 01:35
@home-assistant home-assistant Bot requested a review from frenck March 25, 2026 01:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
Comment thread homeassistant/components/lg_thinq/sensor.py Outdated
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow up @LG-ThinQ-Integration 👍

However, could you add the requested test case above?

Thanks already!

../Frenck

                       

Blogging my personal ramblings at frenck.dev

@home-assistant home-assistant Bot marked this pull request as draft March 25, 2026 06:54
Copilot AI review requested due to automatic review settings March 28, 2026 03:00
@LG-ThinQ-Integration LG-ThinQ-Integration marked this pull request as ready for review March 28, 2026 03:04
@home-assistant home-assistant Bot requested a review from frenck March 28, 2026 03:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread homeassistant/components/lg_thinq/strings.json Outdated
Comment thread homeassistant/components/lg_thinq/strings.json Outdated
Copy link
Copy Markdown
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so I don't want to block this PR per se as this change doesn't add the entities, but I do want to ask the question

Comment thread tests/components/lg_thinq/snapshots/test_sensor.ambr
Comment thread homeassistant/components/lg_thinq/climate.py Outdated
Copilot AI review requested due to automatic review settings March 30, 2026 09:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@LG-ThinQ-Integration
Copy link
Copy Markdown
Contributor Author

The climate has seperated to #166865.

Comment thread tests/components/lg_thinq/snapshots/test_sensor.ambr
@home-assistant home-assistant Bot marked this pull request as draft March 30, 2026 13:38
@LG-ThinQ-Integration
Copy link
Copy Markdown
Contributor Author

LG-ThinQ-Integration commented Mar 31, 2026

@joostlek
This PR is related to a series of changes for kimchi_refrigerator, involving thinqconnect, sensor, and climate.
While attempting to refine the data.value parsing, I noticed an unintended side effect on the fan_mode in the climate platform. To address this, I plan to strengthen exception handling for kimchi_refrigerator within thinqconnect and update the sensor logic to ensure it only impacts the intended device type.

Given the remaining work, I don't believe these changes will be ready for the April 2026 release. Therefore, I'd like to revert the thinqconnect bump for now. Should I submit a separate revert PR, or is there another preferred way to handle this? Please let me know.

Copilot AI review requested due to automatic review settings April 22, 2026 07:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@LG-ThinQ-Integration LG-ThinQ-Integration marked this pull request as ready for review April 22, 2026 08:52
Copilot AI review requested due to automatic review settings April 22, 2026 08:52
@home-assistant home-assistant Bot requested a review from joostlek April 22, 2026 08:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix by-code-owner cla-signed integration: lg_thinq Quality Scale: No score small-pr PRs with less than 30 lines. Top 200 Integration is ranked within the top 200 by usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants