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.

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

Emoncms #25

Merged
merged 1 commit into from
Feb 8, 2014
Merged

Emoncms #25

merged 1 commit into from
Feb 8, 2014

Conversation

henols
Copy link
Contributor

@henols henols commented Feb 7, 2014

A node that can publish to one or more Emoncms servers and acconts

A node that can publish to one or more Emoncms servers and acconts
@knolleary
Copy link
Member

Hi thanks for this.

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.

@henols
Copy link
Contributor Author

henols commented Feb 7, 2014

CLA emailed and posted.

@knolleary
Copy link
Member

Received.

One question, the help text you have for the node is fairly limited:

Performs post to Emoncms.
The Topic is not mandatory, if Topic is left blank <b>msg.topic</b> will used.
The Node is not mandatory, (numberic)

Not being familiar to Emoncms, is that sufficient help for someone relatively new to either Node-RED or Emoncms to understand what this node does and how to use it? Does it need an example set of values?

@henols
Copy link
Contributor Author

henols commented Feb 7, 2014

I might agree with you, but the use of Emoncms is quite simple and my native tung is not English so some times its a litle bit tricky to make your self clear. But i will have a look in to it to see what i can do.

@knolleary
Copy link
Member

@henols no problem at all - your English is far better than my Swedish ;)

I'm happy to merge as it is and we can update it later

knolleary added a commit that referenced this pull request Feb 8, 2014
@knolleary knolleary merged commit 90eef93 into node-red:master Feb 8, 2014
@dceejay
Copy link
Member

dceejay commented Feb 8, 2014

a few thoughts....
We like to have the Name as the last item in the html form can we move the Node field up one.
I don't know the emoncms api - is the Node value always numeric ? and optional ?
On the .js side - which should take priority ? The passed in topic or the configured topic ? (I'm not sure it matters but we will document it)
Should the node(group) also be able to be overwritten in the same way ? by passing in msg.node maybe ?

The API key credentials are stored in the node - we must document this so users are aware in case they "export" their key by mistake - Ideally they should be stored elsewhere - see the mqtt - or mysql nodes for a way to do it - or the prowl, or notify nodes for a less optimum but possibly simpler way.

Raised as Issue #27 so we can track

@knolleary
Copy link
Member

@dceejay - all good comments. As this PR has now been merged, then they should be treated as issues against the emoncms node.

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.

None yet

3 participants