search

MycroftAI / skill-homeassistant

Mycroft Skill/Integration for Homeassistant
GNU Lesser General Public License v3.0
114 stars 62 forks source link

Cannot obtain values from entity #65

Closed adocampo closed 2 years ago

adocampo commented 3 years ago

Describe the bug Cannot obtain values from my Thermostat. I saw the vocab sentences and told mycroft to tell me the values of the current temperature, but it fails

To Reproduce Steps to reproduce the behavior:

  1. Say "Give me the value of thermostat <ENTITY NAME>"
  2. you can try also with "tell me value of <ENTITY NAME>"
  3. See error ' An error occurred while processing a request in Home Assistant Skill'

Expected behavior It should read aloud the current temperature

Log files

 13:51:35.342 | ERROR    | 169044 | mycroft.skills.mycroft_skill.mycroft_skill:on_error:835 | An error occurred while processing a request in Home Assistant Skill
Traceback (most recent call last):
  File "/home/malevolent/mycroft-core/mycroft/skills/mycroft_skill/event_container.py", line 66, in wrapper
    handler(message)
  File "/opt/mycroft/skills/skill-homeassistant/__init__.py", line 187, in handle_sensor_intent
    self._handle_sensor(message)
  File "/opt/mycroft/skills/skill-homeassistant/__init__.py", line 456, in _handle_sensor
    quantity.uncertainty <= 0.5):
TypeError: '<=' not supported between instances of 'NoneType' and 'float'

Environment (please complete the following information):

Tony763 commented 3 years ago

Hello, which version of HA and which language in Mycroft you use?

Could You update to latest version of skill, add log and send me its output.

        sensor_unit = unit_measurement.get('unit_measure') or ''

        sensor_name = unit_measurement['name']
        sensor_state = unit_measurement['state']

        self.log.debug("Unit: %s" % sensor_unit)
        self.log.debug("Name: %s" % sensor_name)
        self.log.debug("State: %s" % sensor_state)
adocampo commented 3 years ago

I'm still using Mycroft on English yet my HA devices are in Spanish. I'm making the tests through Mycroft debug by typing the queries, so all HA queries to switches for instance, are working, but asking for temperature of my sensors doesn't.

System Health

version core-2021.2.2
installation_type Home Assistant Container
dev false
hassio false
docker true
virtualenv false
python_version 3.8.7
os_name Linux
os_version 5.10.5-arch1-1
arch x86_64
timezone Europe/Madrid
Home Assistant Cloud logged_in | false -- | -- can_reach_cert_server | ok can_reach_cloud_auth | ok can_reach_cloud | ok
Lovelace dashboards | 1 -- | -- resources | 0 views | 2 mode | storage

Regarding updating the skill, I went to /opt/mycroft/skills/skill-homeassistant and git pulled, but it said everything was up-to-date. I don't know what to do with the code you gave me, it seems it's python, but I don't know what to do with that. I will be glad to do it if you explain what I need to do with that.

adocampo commented 3 years ago

Just for your information, when I set the temperature on the thermostat, it works, yet the acknowledgment message shows a mysterious "None", but getting that value, it fails

imagen

Tony763 commented 3 years ago

Regarding None in the acknowledgment message, it means that HA did not return a unit of value. Do you have a units visible in HA? Post me a screen of thermostat, please.

adocampo commented 3 years ago

imagen

The big number is the current temperature while the smaller one is the target temperature, so yes, it doesn't show the unit

Tony763 commented 3 years ago

Ok, I will simulate it on my setup tonight and try to write a fix.

adocampo commented 3 years ago

Nice, and regarding this code?

    sensor_unit = unit_measurement.get('unit_measure') or ''

    sensor_name = unit_measurement['name']
    sensor_state = unit_measurement['state']

    self.log.debug("Unit: %s" % sensor_unit)
    self.log.debug("Name: %s" % sensor_name)
    self.log.debug("State: %s" % sensor_state)

Can I do something to help? If you explain to me what I need to do, I'll be glad to help

Tony763 commented 3 years ago

Not needed, it is only for printing values that HA return back to Mycroft. We know that problem is with sensor_unit.

sensor_unit = unit_measurement.get('unit_measure') or '' mean that if returned value is None (dimensionless) it should be filled with a empty string, so you won't get None in response. I have one real and one dummy thermostat so I can simulate it. When I get back from work, I will test it, if I can not simulate it, I will ask you for more details (with step by step).

Tony763 commented 3 years ago

When setting a desired temperature, HA wont send back a units. image

We can ask HA developers to add units to their api. Also if unit value unit is not found it would be better to change unit_measur = None to "<empty string>"so we do not hear None from Mycroft.

For sensor its same. Current sensor accept only sensor and switch category. ha_entity = self._find_entity(entity, ['sensor', 'switch'])

We could add climate but problem is that sensor intent return only one value. So with climate it would return only state and not actual or set temperature. Maybe It would be better to write new intent for it.

When You ask about state of climate, what would You prefer as response? I think about something like:

<entity name> is currently turned off.
<entity name> is currently turned on, set to 20 and actual temperature is 22.
adocampo commented 3 years ago

The On/off status can be stated if you try to set a temperature, if it's off you can't set any temperature (you must first turn it on).

When I ask for the temperature, I want to know the current one. Ideally we should be able to ask both the current temperature and the target temperature (and why not, if the thermostat is on or off), so it should answer for the three questions for separate.

If I need to choose between just one option, I think the <entity name> is currently turned on, set to 20 and current temperature is 22. is the best one, but I think it's too long when we usually just want to know the current temperature, and then set the temperature accordingly.

Tony763 commented 3 years ago

@adocampo Hi, sorry for delay. Lets begin slowly. Could you test this branch https://github.com/Tony763/mycroft-homeassistant/tree/issue65 ?

This should fix case, when no unit is returned from HA. Mycroft should not tell None.

pfefferle commented 3 years ago

I have the same issue with the latest version of mycroft and the home assistant skill. I am using german as language:

 File "/opt/mycroft/mycroft/skills/mycroft_skill/event_container.py", line 73, in wrapper
    handler(message)
  File "/opt/mycroft/skills/homeassistant.mycroftai/__init__.py", line 185, in handle_sensor_intent
    self._handle_sensor(message)
  File "/opt/mycroft/skills/homeassistant.mycroftai/__init__.py", line 487, in _handle_sensor
    quantity.uncertainty <= 0.5):
TypeError: '<=' not supported between instances of 'NoneType' and 'float'
Tony763 commented 3 years ago

Hi @pfefferle just to be sure, is it a latest skill version from marketplace or from this repo?

pfefferle commented 3 years ago

@Tony763 from this repo

Tony763 commented 3 years ago

Hi @pfefferle , this one https://github.com/Tony763/mycroft-homeassistant/tree/issue65 ?

pfefferle commented 3 years ago

yes, I also changed line 128 to return "", but this didn't work out either.

pfefferle commented 3 years ago

@Tony763 I should have given this feedback before proposing another fix! sorry about that :(

Tony763 commented 3 years ago

@Tony763 I should have given this feedback before proposing another fix! sorry about that :(

No problem, not blaming you :slightly_smiling_face: Sorry if i sounded bad.

yes, I also changed line 128 to return "", but this didn't work out either.

If changing line 116 and 128 did not help, problem must be some elsewhere. This piece of code was not used in past versions as quantulum3 was not in requirements. I will open it in debug once again and check. NoneType error should not occur.