Skip to content

Fix EntityComponent handle entities without a name#8065

Merged
balloob merged 2 commits into
devfrom
fix-groups-entity-without-name
Jun 17, 2017
Merged

Fix EntityComponent handle entities without a name#8065
balloob merged 2 commits into
devfrom
fix-groups-entity-without-name

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Jun 16, 2017

Description:

In #7681 we added ordering to groups created by the EntityComponent. That PR incorrectly assumed that all entities always return a name.

To keep consistent ordering, entities have been converted to an ordered dictionary.

CC @amelchio

Related issue (if applicable): Reported by @arraylabs on Gitter:

File "/usr/src/app/homeassistant/helpers/entity_component.py", line 376,
  in async_add_entities yield from self.component.async_update_group()
File "/usr/src/app/homeassistant/helpers/entity_component.py", line 243,
  in async_update_group sorted(self.entities, key=lambda x: self.entities[x].name),
TypeError: '<' not supported between instances of 'NoneType' and 'NoneType'

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@balloob balloob added this to the 0.47 milestone Jun 16, 2017
@amelchio
Copy link
Copy Markdown
Contributor

My bad. I actually did check that assumption but I may have mixed up name and entity_id during my investigation :-(

Anyway, this PR re-introduces the issue #6557 that I tried to fix. The point is to have group.all_scripts and group.all_automations sorted alphabetically for the UI.

I actually do not think the OrderedDict even makes a difference because it seems that entries are added in an arbitrary order during startup(?). So every restart we get a new order anyway.

I propose to instead make the sorting work for None names, maybe like this:

	ids = sorted(self.entities, key=lambda x: self.entities[x].name or x)

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jun 17, 2017

Scripts are based off the configuration, which is parsed into an ordered dict by YAML. So if you want to re-order it, just change the order of the scripts.

@amelchio
Copy link
Copy Markdown
Contributor

That's not what I observed. Unfortunately, I will not have time to test again today.

Getting the lists stable between restarts would be an improvement but I do think alphabetical is a better default ordering and it is also how the default factory view presents things (last I looked, anyway).

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jun 17, 2017

You're right for non-scripts, like platforms. As discovery can always change. I'll pick your solution 👍

@balloob balloob merged commit 1893544 into dev Jun 17, 2017
@balloob balloob deleted the fix-groups-entity-without-name branch June 17, 2017 17:51
@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jun 17, 2017

Cherry-picked for 0.47

balloob added a commit that referenced this pull request Jun 17, 2017
* Fix EntityComponent handle entities without a name

* Implement solution by Anders
@arraylabs
Copy link
Copy Markdown
Contributor

Thanks to all involved! Looking forward to the release today!

@balloob balloob mentioned this pull request Jun 17, 2017
@balloob balloob mentioned this pull request Jul 1, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
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.

6 participants