Skip to content

fix mopar sensor#9389

Merged
balloob merged 3 commits into
home-assistant:devfrom
happyleavesaoc:mopar-fix
Sep 13, 2017
Merged

fix mopar sensor#9389
balloob merged 3 commits into
home-assistant:devfrom
happyleavesaoc:mopar-fix

Conversation

@happyleavesaoc
Copy link
Copy Markdown
Contributor

Description:

mopar sensor was DOA due to 85c98fb. As far as I understand @Throttle, we still have to call update() initially. Also took this opportunity to bump the dependency version to fix a cosmetic bug.

Related issue (if applicable): fixes #9373

Checklist:

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies have been added to requirements_all.txt by running

self.vehicles = []
self.vhrs = {}
self.tow_guides = {}
seld.update()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

undefined name 'seld'

@MartinHjelmare
Copy link
Copy Markdown
Member

The data instance update method should be called once in the entity instance update method before the entity is added to home assistant, cause add_devices has a second argument True. Do you know why that is not enough?

self.vehicles = []
self.vhrs = {}
self.tow_guides = {}
self.update()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

By passing True as second argument to add_devices, Home Assistant will call update() before adding it to Home Assistant. So I'm not 100% sure how this can fix it 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(I do believe you if you say that it fixes it, I just don't understand how)

@happyleavesaoc
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare @balloob This is the MoparData update() method, which is a container for a connection to the web server that fetches data for all mopar sensors (because we maintain a single login session). We have to do an update on that object first, to get a list of vehicles from which we make MoparSensors. Without doing that, no sensors are created (the current state of things).

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Sep 12, 2017

Yes, but the MoparData update method is called from the entity update method, which is done right before the entity is added to home assistant. Why wouldn't that be enough?

Here: https://github.com/happyleavesaoc/home-assistant/blob/4499db663d8dbc3ede1029b721802da91363a54d/homeassistant/components/sensor/mopar.py#L124

@happyleavesaoc
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare The entity can't exist in the first place unless MoparData's update() is called during setup. Note that we iterate data.vehicles to create the list of MoparSensors to use as an argument to add_devices. data.vehicles is a member variable of the data MoparData instance which is populated once the update() method is called. When and where the entities themselves call update() is irrelevant to this issue.

@MartinHjelmare
Copy link
Copy Markdown
Member

That was the missing piece of my puzzle. Thanks @happyleavesaoc! So another approach would be to call data.update() in setup_platform before add_devices, if we don't like to add the update call in the init method of the data class.

But I think your fix is good. 👍

@balloob balloob added this to the 0.53.2 milestone Sep 13, 2017
@balloob balloob merged commit c8da95c into home-assistant:dev Sep 13, 2017
@balloob balloob mentioned this pull request Sep 22, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mopar component, nothing showing up in HA

6 participants