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.

Sign up for GitHub

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

Jump to bottom

Extended map to support geo location entities #2337

Merged
merged 6 commits into from Dec 17, 2018
Merged

Extended map to support geo location entities #2337

merged 6 commits into from Dec 17, 2018

Conversation

exxamalte
Copy link
Contributor

@exxamalte exxamalte commented Dec 16, 2018 •
edited

This is an extension to the lovelace map card that can not only display manually configured entities, but allows for configuring a geo_location_sources array; each geo location platform defines its source. All current geo location entities with that source will be displayed on the map automatically.
Either entities or geo_location_sources is required; if geo_location_sources is defined, then entities is optional.

Sample card configuration:

type: map
geo_location_sources:
  - nsw_rural_fire_service_feed
entities:
  - zone.home
default_zoom: 9

lovelace-geo-location-map

Pull request in home-assistant.io: home-assistant/home-assistant.io#7880

@ghost ghost added the in progress label Dec 16, 2018
@exxamalte
Copy link
Contributor Author

Hold on, I just realised that entities is required, but instead source should be required and entities optional.

@iantrich
Copy link
Member

We have a map card. Why would you not at least add this source to the existing rather than copy all its code to a new file to add it?

@exxamalte
Copy link
Contributor Author

exxamalte commented Dec 16, 2018 •
edited

Sure, happy to do that as well. I'm just wondering how to untangle which config parameters are then required and which ones are optional: The default map requires entities to be set, while the geo location map requires source to be set and additional entities are optional.

How about: If source is set, then entities are optional?

@exxamalte
Copy link
Contributor Author

exxamalte commented Dec 16, 2018 •
edited

Alright, I merged the geo location code into the existing map.

Sorry, accidentally closed the PR, reopened straight away...

@exxamalte exxamalte closed this Dec 16, 2018
@ghost ghost removed the in progress label Dec 16, 2018
@exxamalte exxamalte reopened this Dec 16, 2018
@ghost ghost added the in progress label Dec 16, 2018
@exxamalte exxamalte changed the title Geo Location Map Extended map to support geo location entities Dec 16, 2018
@balloob
Copy link
Member

balloob commented Dec 16, 2018

Geo location sources should probably be an array?

@exxamalte
Copy link
Contributor Author

Geo location sources should probably be an array?

Good point, because that would be the only way to join entities from different geo location platforms into one map.

balloob
balloob requested changes Dec 17, 2018
View reviewed changes
src/panels/lovelace/cards/hui-map-card.js Outdated
@@ -205,7 +222,24 @@ class HuiMapCard extends PolymerElement {
}
const mapItems = (this._mapItems = []);

this._configEntities.forEach((entity) => {
var allEntities = [];
Copy link
Member

Choose a reason for hiding this comment

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

Doing use var

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will replace with let

src/panels/lovelace/cards/hui-map-card.js Outdated
@@ -111,7 +111,24 @@ class HuiMapCard extends PolymerElement {
throw new Error("Error in card configuration.");
}

this._configEntities = processConfigEntities(config.entities);
this._configGeoLocationSources = config.geo_location_sources;
Copy link
Member

Choose a reason for hiding this comment

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

Don't assign before raising errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, moving code around.

@exxamalte exxamalte mentioned this pull request Dec 17, 2018
Merged
2 tasks
@balloob balloob merged commit cac7f8d into home-assistant:dev Dec 17, 2018
@ghost ghost removed the in progress label Dec 17, 2018
@balloob
Copy link
Member

balloob commented Dec 17, 2018

Do you want to submit a PR to add this functionality to a map demo ?

@exxamalte
Copy link
Contributor Author

Do you want to submit a PR to add this functionality to a map demo ?

Sure. I'll take a look at that now...

@exxamalte exxamalte deleted the geo-location-map branch December 17, 2018 11:44
@frenck
Copy link
Member

frenck commented Dec 30, 2018

Just a note @exxamalte, geolocation is one word. So, it should be geolocation_sources...

@thomasloven
Copy link
Contributor

@exxamalte I cannot find any documentation for this. Do you have something prepared, or should I add it when I'm documenting #2489 ?

@exxamalte
Copy link
Contributor Author

@thomasloven The documentation for this PR can be found here: home-assistant/home-assistant.io#7880

@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@balloob balloob balloob approved these changes

Assignees
No one assigned
Labels
cla-signed Documentation Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@exxamalte @iantrich @balloob @frenck @thomasloven @homeassistant