Skip to content

Heatmiser feature#44

Merged
knolleary merged 2 commits intonode-red:masterfrom
bedfordsean:heatmiser-feature
Apr 16, 2014
Merged

Heatmiser feature#44
knolleary merged 2 commits intonode-red:masterfrom
bedfordsean:heatmiser-feature

Conversation

@bedfordsean
Copy link
Copy Markdown

Heatmiser node addition. This node reads/writes to a Heatmiser thermostat on a specified IP with a specified PIN.

By default it is configured to poll the thermostat every 30 minutes and send an update. This is configurable in the node settings.

@knolleary
Copy link
Copy Markdown
Member

Hi, some comments

  1. we don't generally have nodes that have both inputs and outputs, but don't sit in the middle of a flow. It may be better to split this into two separate nodes - one for setting data, and one for querying/reporting data.
  2. you use setInterval, but you don't clearInterval anywhere. This will cause additional intervals to be set each time you deploy. You need to store the id returned by setInterval, and then listen for the close event on the node to clear the interval. Have a look at, for example, the Delay node for an example of this.
  3. there are some console.logs in there which should be removed
  4. copyright statements should be you not IBM :)
  5. please squash your commits down - here's a guide for doing that

Finally, before we can merge this, we need you to complete a CLA as described in our README.

You can download the CLA from here: http://nodered.org/cla/node-red-cla-individual.pdf

Please print, complete, sign, scan and email to me (details in the document). Please add a comment here as well once emailed.

@bedfordsean bedfordsean deleted the heatmiser-feature branch March 24, 2014 20:16
@bedfordsean bedfordsean reopened this Mar 24, 2014
@bedfordsean
Copy link
Copy Markdown
Author

Hi Nick

Now squashed into two commits.
I've updated the copyright, and split out into an input and an output node.
I've also addressed the clearInterval issue and removed the console.log statements.

I have also emailed you a signed copy of the CLA.

Squashed commits:
[3079c2d] Added Heatmiser input and output nodes
[62bd1f3] Added Heatmiser input and output nodes

Fix input node bugs
@bedfordsean bedfordsean reopened this Mar 24, 2014
@knolleary
Copy link
Copy Markdown
Member

node-red-gitbot: please check cla status

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants