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

APC UPS Daemon sensor entities have spaces at the end of numerical states #81476

Closed
SteveDinn opened this issue Nov 3, 2022 · 11 comments · Fixed by #93783
Closed

APC UPS Daemon sensor entities have spaces at the end of numerical states #81476

SteveDinn opened this issue Nov 3, 2022 · 11 comments · Fixed by #93783

Comments

@SteveDinn
Copy link
Contributor

The problem

I added the integration for my APC UPS and I was presented with 8 entities:

image

However, when I examine one of these entities, and look at the 'Related' tab, I see many, many more. Shouldn't these all be listed on the integration page?

image

What version of Home Assistant Core has the issue?

2022.11.0

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

APC UPS Daemon

Link to integration documentation on our website

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

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

@home-assistant
Copy link

home-assistant bot commented Nov 3, 2022

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

Code owner commands

Code owners of apcupsd can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign apcupsd Removes the current integration label and assignees on the issue, add the integration domain after the command.

(message by CodeOwnersMention)


apcupsd documentation
apcupsd source
(message by IssueLinks)

@SteveDinn
Copy link
Contributor Author

I don't know what's up with this, but I came back into that integration page about 5 minutes after I logged this bug, and all the entities are there now. But man, I took screenshots :) There were only 8 when I added it.

@SteveDinn
Copy link
Contributor Author

I'm going to tag onto this bug with this issue too, because I don't feel like opening up other issues:

States of the entities seem to have trailing spaces, and are interpreted as strings, even for numeric values
e.g.:

<template TemplateState(<state sensor.ups_input_voltage=121.6 ; unit_of_measurement=V, icon=mdi:flash, friendly_name=UPS Input Voltage @ 2022-11-03T09:55:55.697300-03:00>)>
<template TemplateState(<state sensor.ups_nominal_output_voltage=120 ; unit_of_measurement=V, icon=mdi:flash, friendly_name=UPS Nominal Output Voltage @ 2022-11-03T09:43:25.500504-03:00>)>

This makes it weird when you try to format the value with its unit_of_measurement. value like

{{ state('sensor.ups_input_voltage') }}{{ state_attr('sensor.ups_input_voltage', 'unit_of_measurement') }}

and more importantly, can't be used by numeric_state triggers without some massaging.

@SteveDinn
Copy link
Contributor Author

IT seems to be only the numerical values that have this issue. The string and date formatted entity states seem OK.

{% for state in states if (state.entity_id.startswith('sensor.ups_')) -%}
  {{ state.entity_id }} value quoted: "{{ state.state }}" {{"\n"}}
{%- endfor -%}
sensor.ups_alarm_delay value quoted: "30 " 
sensor.ups_battery value quoted: "100.0 " 
sensor.ups_battery_nominal_voltage value quoted: "24.0 " 
sensor.ups_battery_replaced value quoted: "2021-03-12" 
sensor.ups_battery_shutdown value quoted: "5 " 
sensor.ups_battery_timeout value quoted: "0 " 
sensor.ups_battery_voltage value quoted: "27.5 " 
sensor.ups_input_voltage value quoted: "121.6 " 
sensor.ups_internal_temperature value quoted: "29.2 " 
sensor.ups_line_frequency value quoted: "60.0 " 
sensor.ups_load value quoted: "27.9 " 
sensor.ups_mode value quoted: "Stand Alone" 
sensor.ups_nominal_output_voltage value quoted: "120 " 
sensor.ups_output_voltage value quoted: "121.6 " 
sensor.ups_restore_requirement value quoted: "0.0 " 
sensor.ups_self_test_interval value quoted: "7 days" 
sensor.ups_self_test_result value quoted: "NO" 
sensor.ups_shutdown_delay value quoted: "270 " 
sensor.ups_shutdown_time value quoted: "3 " 
sensor.ups_startup_time value quoted: "2022-11-03 09:41:17 -0300" 
sensor.ups_status value quoted: "ONLINE" 
sensor.ups_time_left value quoted: "40.0 " 
sensor.ups_time_on_battery value quoted: "0 " 
sensor.ups_total_time_on_battery value quoted: "0 " 
sensor.ups_transfer_count value quoted: "0" 
sensor.ups_transfer_from_battery value quoted: "N/A" 
sensor.ups_transfer_high value quoted: "127.0 " 
sensor.ups_transfer_low value quoted: "106.0 " 
sensor.ups_wake_delay value quoted: "-1 "

It would also be cool if the ups_self_test_interval sensor could report in some numerical format instead of the textual-number-with-unit-format that it currently does.

@borpin
Copy link

borpin commented Nov 28, 2022

@yuxincs, I suspect the issue is this line (though I could be wrong)

return value[: -len(unit)], INFERRED_UNITS.get(unit, unit.strip())

As the string from NIS is LINEV : 232.0 Volts

So (I think) the code is only removing the length of the unit, not the space as well from the input string. I'm surprised there isn't a type conversion somewhere as well (to int or float).

Also looks like it needs a unit named days as well.

Good catch @SteveDinn.

@yuxincs
Copy link
Contributor

yuxincs commented Jan 17, 2023

Nice catch, all!

I noticed this bug as well when I was adding tests to prevent regressions for future development. That indeed seems to be causing the problems here.

I'll find some time to fix this soon, but if any of you like to create a PR, please do 😃

@issue-triage-workflows
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@SteveDinn
Copy link
Contributor Author

This is still an issue. Back off issue-triage-workflows bot.

@SteveDinn SteveDinn changed the title APC UPS Daemon integration doesn't show all entities APC UPS Daemon sensor entities have spaces at the end of numerical states Apr 17, 2023
@github-actions github-actions bot removed the stale label Apr 17, 2023
@yuxincs
Copy link
Contributor

yuxincs commented May 30, 2023

So, I think this issue is fixed by #93724 as well, which properly removes the leading spaces when splitting the numeric value and the unit from the raw strings 😃

@borpin
Copy link

borpin commented May 30, 2023

Well a fix is proposed. I'm not holding my breath for the Devs to accept it :)

@yuxincs
Copy link
Contributor

yuxincs commented May 30, 2023

The PR is actually already merged, I believe you'll see it working in next release several days later :)

@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants