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

Expand ZHA configuration panel #2421

Merged
merged 33 commits into from Jan 11, 2019

Conversation

Projects
None yet
4 participants
@dmulcahey
Copy link
Contributor

dmulcahey commented Jan 7, 2019

don't merge before home-assistant/home-assistant#19664

screen_shot_2019-01-06_at_3 49 07_pm
screen_shot_2019-01-06_at_3 49 26_pm

dmulcahey added some commits Jan 5, 2019

dmulcahey added some commits Jan 9, 2019

Show resolved Hide resolved src/data/zha.ts Outdated
Show resolved Hide resolved src/data/zha.ts Outdated
Show resolved Hide resolved src/data/zha.ts Outdated
Show resolved Hide resolved src/data/zha.ts Outdated
Show resolved Hide resolved src/data/zha.ts Outdated
Show resolved Hide resolved src/data/zha.ts Outdated

export class HaConfigZha extends LitElement {
public hass?: HomeAssistant;
public isWide?: boolean;
private _haStyle?: DocumentFragment;
private _ironFlex?: DocumentFragment;
private _selectedNode?: HassEntity;

This comment has been minimized.

@balloob

balloob Jan 10, 2019

Member

Do all nodes map to an entity in HA ?

Are these 3 selected things mutually exclusive or can they co-exist?

This comment has been minimized.

@dmulcahey

dmulcahey Jan 10, 2019

Author Contributor

This is a nuanced answer... maybe better suited for discord. Short answer: node is essentially device. It can have 1 or many entities. Each entity can have 1 or many clusters. each cluster can have 1 or many commands and 1 or many attributes.

This comment has been minimized.

@balloob

balloob Jan 11, 2019

Member

Okay, what about a remote, that won't have an entity but just generate events, no?

This comment has been minimized.

@balloob

balloob Jan 11, 2019

Member

Would it make sense to just fetch all nodes via the websocket command and use that ? Because now you might end up adding more things to entities to satisfy a config panel.

This comment has been minimized.

@balloob

balloob Jan 11, 2019

Member

And I also suggest that you make a selection type that contains all three selections.

type ZHASelection {
  node: string;
  cluster: string;
}

This comment has been minimized.

@dmulcahey

dmulcahey Jan 11, 2019

Author Contributor

the selections are mainly propagated to clear selections that aren't valid when another is changed... does that make sense? Everything is built off of drilling down.

Ex: I Pick a node -> pick an entity -> pick a cluster -> pick an attribute.

If I subsequently change the entity I have selected I then have to clear the selected cluster and attribute that I had already selected because the selections aren't valid and the data in each picker changes.

I am using events to propagate information up the dom and bindings to propagate data down the dom so that independent components can interact. Did I do something wrong here?

I don't see what ZHASelection gives me here esp since the components that fire events may not have all of the parts to populate it correctly. Am I making sense? Also, not challenging / arguing. Just making sure we're on the same page and that I understand what you mean.

@balloob

This comment has been minimized.

Copy link
Member

balloob commented Jan 10, 2019

So eh, what happened to one card at a time? 😉

@dmulcahey

This comment has been minimized.

Copy link
Contributor Author

dmulcahey commented Jan 11, 2019

So eh, what happened to one card at a time? 😉

I'm sorry. I'm a bad person :(
I promise I won't do it like this again :)

const serviceData = {
type: "zha/entities/clusters/attributes/value",
};
Object.assign(serviceData, data);

This comment has been minimized.

@balloob

balloob Jan 11, 2019

Member

This is not needed. Did you see my example code in my previous comment?

hass.callWS({
  ...data,
  type: "zha/entities/clusters/attributes/value",
})

This comment has been minimized.

@dmulcahey

dmulcahey Jan 11, 2019

Author Contributor

Whoops I’ll fix it

clusterId: number,
clusterType: string
): Promise<Command[]> =>
hass!.callWS({

This comment has been minimized.

@balloob

balloob Jan 11, 2019

Member

No ! needed, as hass is not scoped as optional

@balloob balloob merged commit 6d43c9e into home-assistant:dev Jan 11, 2019

4 checks passed

WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@wafflebot wafflebot bot removed the in progress label Jan 11, 2019

@balloob balloob referenced this pull request Jan 17, 2019

Merged

20190116.0 #2493

@Hedda Hedda referenced this pull request Jan 24, 2019

Merged

0.86.0 #20354

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment