Skip to content

[WIP] Zwave panel#260

Merged
balloob merged 43 commits into
home-assistant:masterfrom
turbokongen:zwave-panel
May 19, 2017
Merged

[WIP] Zwave panel#260
balloob merged 43 commits into
home-assistant:masterfrom
turbokongen:zwave-panel

Conversation

@turbokongen
Copy link
Copy Markdown
Contributor

@turbokongen turbokongen commented Apr 16, 2017 •
edited
Loading

For home-assistant/core#7127
This is an open PR so anybody can check in and comment. By no means finished or polished.
Services available:

  • Add Node(secure)
  • Remove Node
  • Cancel command
  • Heal Network
  • Start network
  • Stop Network
  • Soft reset
  • Test network
  • Set config parameter
  • Print config parameter
  • Remove failed node
  • Replace failed node
  • Change association
  • Set Wakeup
  • Print node
  • Refresh entity
  • Refresh node
  • view of OZW_Log.txt
  • node group members
  • Node information (zwave device entity attributes)
  • set_usercode
  • clear_usercode
    image

@turbokongen turbokongen mentioned this pull request May 5, 2017
Merged
@balloob
Copy link
Copy Markdown
Member

balloob commented May 9, 2017

Turbo, a 1000 lines component ?! Can you please break it up. Preferably just do 1 card per component.

@balloob
Copy link
Copy Markdown
Member

balloob commented May 9, 2017

Also MVP haha, next time we should do 1 feature at a time. Makes code review a lot easier

balloob
balloob reviewed May 9, 2017
View reviewed changes
Comment thread panels/zwave/ha-panel-zwave.html
hass='[[hass]]'
domain='zwave'
service='refresh_node'
service-data=[[computeNodeServiceData(selectedNode)]]
Copy link
Copy Markdown
Member

@balloob balloob May 9, 2017

Choose a reason for hiding this comment

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

If you're going to compute the same service data many times, put it in a computed property so it is only computed once.

@turbokongen
Copy link
Copy Markdown
Contributor Author

turbokongen commented May 13, 2017 •
edited
Loading

So, I have exported all cards, leaving only the main node card, reducing the main panel to 433 lines.
I have left all the logging in, so I can adjust for review changes. Will be removed in the end.

@balloob
Copy link
Copy Markdown
Member

balloob commented May 16, 2017

Thanks @turbokongen . I will scan over it but won't go in depth on the review, ain't nobody got time for that

balloob
balloob approved these changes May 16, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

It looks all good. 🐬 Super stoked for this!

Ok to merge when logging statements have been removed and linting addressed https://travis-ci.org/home-assistant/home-assistant-polymer/builds/231836811#L247

balloob
balloob reviewed May 16, 2017
View reviewed changes
Comment thread panels/zwave/ha-panel-zwave.html Outdated
},

computeNodeNameServiceData: function (newNodeNameInput) {
// console.log('Rename node:', JSON.stringify({ node_id: this.nodes[this.selectedNode].attributes.node_id,
Copy link
Copy Markdown
Member

@balloob balloob May 16, 2017

Choose a reason for hiding this comment

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

For the future, you don't want to stringify before logging. That way you can actually play with the object.

@turbokongen
Copy link
Copy Markdown
Contributor Author

turbokongen commented May 16, 2017 •
edited
Loading

Wow! Really? I was super prepared for a good piece of work with review changes. 😃
Anyway, I have a little quirk with the set-config service, so I am reworking how it is being used in the backend. The value is different than the content, so It needs to be set as string on list parameters. As soon as I have it working and tests are modifyed I'll submit a PR for the backend for that, and clean up the frontend code. Then we are ready to go!!! It's been a good piece of work.

@balloob balloob merged commit ad3b3ce into home-assistant:master May 19, 2017
@bramkragten bramkragten mentioned this pull request Jan 29, 2020
Merged
tkdrob pushed a commit to tkdrob/frontend that referenced this pull request Apr 20, 2021
@TomBrien
Add registration last update trigger (home-assistant#260)
c7a8117 
I noticed we have this as a possible value of `sensor.xxx_last_update_trigger`
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 8, 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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@turbokongen @balloob @homeassistant