-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add design note on GPIO node enhancements #29
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you have an existing (basic mode) node set to pin 7 and this send an array that includes pin 7 ?
I still think that the whole advanced modes of operation of this proposal are very significantly different from the basic mode, and should be in a separate "professional" node rather than added to this one.
It would also allow the node to be based on a different underlying library that may be more performant that the current pipe to python script, which would help PWM and also allow servo timing modes that the current node cannot support.
The existing one is no longer part of the core set and so can either not be installed or can easily be removed.
|
||
1. Add advanced mode checkbox. | ||
2. In advanced mode, the **GPIO in** node has input port and can receive a payload value: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be sent dynamically ? IE can the pin number be changed often ? Currently there is also option to read initial value ? Would that happen on every pin change ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The pin number is a required parameter in current proposal.
I don't expect to read the initial value without input message in advanced mode.
| `pin` | int | + | GPIO pin number | | ||
| `delay` | number | - | delay after input in ms | | ||
| `wait` | number | - | wait befor input in ms | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these modes are read on demand ? rather than edge detect ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once an input message has been accepted, I expect the edge detect behavior before next message is received.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if there are several rapid changes - how would this "interact" with e delay and wait times ? (This isn't debounce as that is specified in another parameter below) - so if an edge occurs during the wait time what happens ? - or during the wait time ? Does it discard ? - form a queue ? reset the timers ? only send last good value ?
} | ||
``` | ||
|
||
3. In advanced mode, **GPIO out** node has an output port. The **GPIO out** node outputs a message containing payload with `true` value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the same as done ? ie triggered at end of a sequence... ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Since the flow using done
is not easy to understand the execution ordering of nodes, I want to make the connection relation of the nodes explicit.
Another comment. When you select "advanced mode" - then presumably the existing pin selection UI would need to disappear - as selecting single pins is not valid anymore - and be replaced by what ? I think that would be simply too jarring for users. It is not as-if you can switch and have your existing settings carried forwards - and vice versa back again... it really is a different node. We do already have "other" gpio nodes like the node-red-node-pi-liter which is a simple node to drive a strip of 8 leds - but has various other modes built in - like bar graph, single led pointer, byte mode, etc... and we also have the pigpiod node, so other gpio like nodes are allowed in the nodes repo :-) |
Thank you for your review comment.
The current assumption is that unexpected input will cause an error.
I understand your opinion.
Yes. I think moving to a better library is one way to go. However, as I mentioned before, RPi.GPIO can also be used on other platforms that we use, so it would be nice if we could make it selectable. |
As for the UI, you're right, I'm currently envisioning hiding the settings as seen on other nodes, or greying them out to make them unselectable. |
My worry is that this is not a simple extension of the node - as soon as you turn on the advanced behaviour - it is
To me it would be like having someone learn to drive a motor scooter then switching it for a large lorry - yes they both have wheels and go on the road, but the controls are different and a lot more complexity. There should be no problem having both nodes installed if you really wanted to. We could certainly add a pointer to the more advanced node from the existing one if needed. |
The GPIO node provide an easy way to control hardware using Node-RED.
However, there are some cases where it is difficult to control certain types of hardware using current GPIO node.
Therefore, this proposal aims to extend the GPIO node to meet such requirements.