Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modbus - Exception since 2023.5.0: state class None: numeric value, however, it has the non-numeric value. #93297

Closed
String-656 opened this issue May 20, 2023 · 13 comments · Fixed by String-656/core#1 or #93832

Comments

@String-656
Copy link
Contributor

The problem

The issue is introduced in 2023.5, without any config changes

ValueError: Sensor sensor.inverter_apparent_power has device class None, state class None unit VA and suggested precision None thus indicating it has a numeric value; however, it has the non-numeric value: nan (<class 'str'>)

What version of Home Assistant Core has the issue?

2023.5.0

What was the last working version of Home Assistant Core?

2023.4.6

What type of installation are you running?

Home Assistant OS

Integration causing the issue

Modbus

Link to integration documentation on our website

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

Diagnostics information

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/modbus/sensor.py", line 137, in async_update
    self.async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 585, in async_write_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 649, in _async_write_ha_state
    state = self._stringify_state(available)
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 591, in _stringify_state
    if (state := self.state) is None:
  File "/usr/src/homeassistant/homeassistant/components/sensor/__init__.py", line 583, in state
    raise ValueError(
ValueError: Sensor sensor.inverter_apparent_power has device class None, state class None unit VA and suggested precision None thus indicating it has a numeric value; however, it has the non-numeric value: nan (<class 'str'>)

Example YAML snippet

type: tcp
host: 172.24.xxx.xxx
port: 502
close_comm_on_error: false
delay: 5
timeout: 5
name: "SolarData"
sensors:
  - name: Inverter Apparent Power
    slave: 1
    address: 40095
    input_type: holding
    unit_of_measurement: "VA"
    count: 2
    precision: 1
    data_type: float32
    scan_interval: 300

Anything in the logs that might be useful for us?

No response

Additional information

Modbus component needs to "be adjusted to strip nan values."

#92483 (comment)

@home-assistant
Copy link

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

Code owner commands

Code owners of modbus 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 modbus Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


modbus documentation
modbus source
(message by IssueLinks)

@janiversen
Copy link
Member

as pr documentation please supply a debug log, so we have a chance of seeing what is happening.

Apart from that I suspect this could be caused by an outside change made by the core team, can you please confirm in which version it worked. There have not been changes to the modbus integration in the latest releases.

@String-656
Copy link
Contributor Author

String-656 commented May 21, 2023

Thanks, I've enabled debug for Modbus, but there's not much additional info in the logs, see below for a full copy/paste.

As above, it was working with 2023.4.6, but broke with 2023.5.0

logger:
  default: warning
  logs:
    homeassistant.components.modbus: debug
    pymodbus: debug
2023-05-21 09:16:32.749 INFO (SyncWorker_2) [homeassistant.components.modbus.modbus] modbus WoonanInverter communication open
2023-05-21 09:16:49.471 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/sensor/__init__.py", line 579, in state
    numerical_value = int(value)
ValueError: invalid literal for int() with base 10: 'nan'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/modbus/sensor.py", line 137, in async_update
    self.async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 585, in async_write_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 649, in _async_write_ha_state
    state = self._stringify_state(available)
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 591, in _stringify_state
    if (state := self.state) is None:
  File "/usr/src/homeassistant/homeassistant/components/sensor/__init__.py", line 583, in state
    raise ValueError(
ValueError: Sensor sensor.inverter_dc_current has device class None, state class None unit A and suggested precision None thus indicating it has a numeric value; however, it has the non-numeric value: nan (<class 'str'>)

Yes, seems a breaking change to the core, many integrations are having this issue. Refer to this post..

Here's another integration fixing the issue.

Let me know if I can assist further / with testing.

@janiversen
Copy link
Member

Just checked the code, the modbus integration do not produce the "nan", that is a conversion in core, when native_value is None (which happens when there are e.g. read errors).

I have no idea how to solve this at integration level, converting None to 0 is for many configurations just false, I think it needs to be solved at core level.

@String-656
Copy link
Contributor Author

String-656 commented May 23, 2023

so.. after some tests with Modbus poll.. I've found that my registers from my inverter are returning "NaN" for floats32. I've also noticed I'm getting these errors at night only, when my solar is not producing any power. No issues or errors when the sun is out....

Note, there's no Modbus exception code, I'm getting "NaN" shown in the Modbus poll field when displaying the response as float 32!.. see below.. I've also included displaying the result in HEX.

Nan - When inverter is sending no value

Ascii - Hex value..
image

So it seems modbus floats can return NaN... and the core is throwing the error?

Could the modbus integration replace "nan" with {{ None }}"?

Update...
just for fun, converted the raw binary from the inverter to confirm.. it's a legit binary read from the device, returning 'NaN'
image

@janiversen
Copy link
Member

You have a very interesting idea, actually there is a pull request open that does something similar, but I think the developer abandoned it.

It would be easy for the modbus integration to make the conversion to None, I will take a look at it later.

Thanks for the analysis.

@String-656
Copy link
Contributor Author

String-656 commented May 24, 2023

I've done little to zero python, only C# and industrial control systems.. but.. might be able to do something like this?.. please ignore if incorrect. I'm unable to test it for now, I'll have a go on the weekend (no dev environment setup, and the custom_component override is not working)

def a function to check if NaN is equal to its self, as it never is... and other python methods need to use imports, like import Math

        def isNaN(num):
            return num != num

and I think this is where you're reading in the registers before passing them off... but just a guess.
sensor.py line 131.

                """Check for NaN"""
                if isNaN(result_array[0]):
                    self._attr_native_value = None
                    if self._coordinator:
                        self._coordinator.async_set_updated_data(None)
                else:
                    self._attr_native_value = result_array[0]
                    self._coordinator.async_set_updated_data(result_array)

image

@janiversen
Copy link
Member

I am not sure what you want to tell with these lines of code, if you believe that is a fix, then you need to submit a PR, and if you want to teach us, well that is really not needed (at least not at that level).

I will take a deeper look, but it will be later, since I have a couple of others to help first.

@String-656
Copy link
Contributor Author

... I was just trying to help.. no need to be condescending. In the unlikely event the above works I'll submit a PR request.

@janiversen
Copy link
Member

I actually just was factual but read your comment as "teaching".

Looking forward to see a PR, which I can review.

@String-656
Copy link
Contributor Author

String-656 commented May 29, 2023

Apologies, that was not my intent. Just thinking out loud.
So.. after stuffing around and getting a dev environment working, I've managed to remove the error and detect / handle nan

It seems this is defaulting to a string value when the floats are returning 'nan'

sensor.py
line 119 result = self.unpack_structure_result(raw_result.registers)

Adding the following after result is set handles the issue before an exception is thrown..

       if result == "nan":
            _LOGGER.debug("String_656 NaN found")
            if self._lazy_errors:
                self._lazy_errors -= 1
                return
            self._lazy_errors = self._lazy_error_count
            self._attr_available = False
            self._attr_native_value = None
            if self._coordinator:
                self._coordinator.async_set_updated_data(None)
            self.async_write_ha_state()
            return

I've never done a PR.. so I'll have a read up on it and submit it tomorrow.

Update... Maybe 'nan' should be handled in unpack_structure_result, as it is returning a string ...
I will do additional testing.

String-656 added a commit to String-656/core that referenced this issue May 30, 2023
home-assistant#93297

Replace NaN with None in unpack_structure_result
@String-656
Copy link
Contributor Author

String-656 commented May 30, 2023

I made a mistake on the first PR. re-did another.. not sure what has happened.. apologies..

The code works.. any guidance on what I did wrong regarding the PR would be appreciated...

@String-656
Copy link
Contributor Author

Third time's a charm.. I think it's ready to be reviewed. Thanks

@github-actions github-actions bot locked and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.