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

Parsing XML with Rest Sensor broken #92836

Closed
mutasim opened this issue May 9, 2023 · 9 comments · Fixed by #93396
Closed

Parsing XML with Rest Sensor broken #92836

mutasim opened this issue May 9, 2023 · 9 comments · Fixed by #93396
Assignees

Comments

@mutasim
Copy link

mutasim commented May 9, 2023

The problem

Parsing an endpoint with the following XML payload:

<cmd status="ok"> <device id="0"> <reg vid="124" tid="heat" v="4" min="0" max="65535"/> </device> </cmd>

using the following template:
value_template: '{{value_json.cmd.device.reg["@v"]}}'

used to correctly extract the 'v' value. Now the sensor shows as 'Unknown'.

What version of Home Assistant Core has the issue?

core-2023.5.0

What was the last working version of Home Assistant Core?

core-2023.4.6

What type of installation are you running?

Home Assistant Container

Integration causing the issue

No response

Link to integration documentation on our website

https://www.home-assistant.io/integrations/sensor.rest/

Diagnostics information

No response

Example YAML snippet

rest:
authentication: basic
  username: "root"
  password: "root"
  scan_interval: 30
  resource: http://10.0.11.22/getregister.cgi?124@heat
  sensor:
    - name: "Open Plan Heat - V Value"
      unique_id: open_plan_heat_v_value
      value_template: '{{value_json.cmd.device.reg["@v"]}}'

Anything in the logs that might be useful for us?

Template variable warning: 'dict object' has no attribute '@v' when rendering '{{value_json.cmd.device.reg["@v"]}}'

Additional information

XML file attached: getregister.xml.zip

No response

@home-assistant
Copy link

home-assistant bot commented May 9, 2023

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

Code owner commands

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

(message by CodeOwnersMention)


rest documentation
rest source
(message by IssueLinks)

@mutasim
Copy link
Author

mutasim commented May 12, 2023

Here is output of the {{value_json}} template for both versions:

2023.4.6:
{'cmd': {'@status': 'ok', 'device': {'@id': '0', 'reg': {'@vid': '124', '@tid': 'heat', '@v': '4', '@min': '0', '@max': '65535'}}}}

2023.5.x
{'cmd': {'@status': 'ok', 'device': {'@id': '0', 'reg': {'@vid': '0', '@tid': '124%40heat', '@status': 'not_found'}}}}

@holysoles
Copy link
Contributor

holysoles commented May 14, 2023

Hey @mutasim can you turn on debug logging by setting this in your configuration.yaml file?

logger:
  default: info
  logs:
    homeassistant.components.rest: debug

There is a debug statement here that will write out the data being returned from your REST endpoint, and another one here. That will confirm how the XML is being converted from JSON.

From what its worth though, i cannot reproduce this on v2023.6.0 (dev branch) . I mocked the REST response to be the XML data you provided, and it still gets parsed correctly. I suspect the data being returned from the endpoint may have changed.

2023-05-14 01:59:57.597 DEBUG (MainThread) [homeassistant.components.rest] Finished fetching rest data data in 0.263 seconds (success: True)
2023-05-14 01:59:57.597 DEBUG (MainThread) [homeassistant.components.rest.sensor] Data fetched from resource: <cmd status="ok"><device id="0"><reg vid="124" tid="heat" v="4" min="0" max="65535"/></device></cmd>
2023-05-14 02:00:01.321 DEBUG (MainThread) [homeassistant.components.rest.sensor] JSON converted from XML: {"cmd":{"@status":"ok","device":{"@id":"0","reg":{"@vid":"124","@tid":"heat","@v":"4","@min":"0","@max":"65535"}}}}

@mutasim
Copy link
Author

mutasim commented May 14, 2023

Hey,

Thanks for the follow up. Looks like the data returned differs depending on which version I run. I've swapped out the container but config & endpoint/payload remain the same.

2023.5.2

DEBUG (MainThread) [homeassistant.components.rest.sensor] Data fetched from resource: <cmd status="ok"><device id="0"><reg vid="0" tid="124%40heat" status="not_found"/></device></cmd>

2023.4.6

DEBUG (MainThread) [homeassistant.components.rest.sensor] Data fetched from resource: <cmd status="ok"><device id="0"><reg vid="124" tid="heat" v="6" min="0" max="65535"/></device></cmd>

For good measure I've uploaded the XML file again that is returned when I access the endpoint in my browser.
getregister.xml.zip

Thanks for your help.

@epenet
Copy link
Contributor

epenet commented May 14, 2023

I guess the issue is with encoding 124@heat in the url:

  • 2023.4 => vid="124" tid="heat"
  • 2023.5 => vid="0" tid="124%40heat"

@holysoles
Copy link
Contributor

@mutasim thanks for that, looks like you ran into a bug in the httpx library, which recently got fixed in this PR encode/httpx#2701

I confirmed that fixes the issue, it encodes the URL correctly with their changes

image

@holysoles
Copy link
Contributor

Taking a closer look at the code over there, I don't think there is an available workaround for this that will cause the @ to not get URL encoded.

I'm not sure when they will do their next official release, but we should bump the version dependency here once they do.

@epenet epenet added the waiting-for-upstream We're waiting for a change upstream label May 14, 2023
@epenet epenet removed the waiting-for-upstream We're waiting for a change upstream label May 23, 2023
@epenet epenet mentioned this issue May 23, 2023
20 tasks
@epenet
Copy link
Contributor

epenet commented May 23, 2023

It would be great if you could test #93396

@mutasim
Copy link
Author

mutasim commented May 23, 2023

Happy to help test - I'll need some guidance on where I can find/how to build this into a docker image.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 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