search

neilenns / node-red-contrib-wled2

NodeRed node for controlling WLED
MIT License
19 stars 8 forks source link

Segment Selection via integer #95

Closed marc-gist closed 3 years ago

marc-gist commented 3 years ago

Fixes #94

Description of changes

Add the ability to select/override the Segment ID for WLEDs with multiple segments. Can set via GUI or in payload

Also added a debug flag so that the payload sent to WLED can be sent to the console via node.warn to aid in debugging issues with WLED not doing what the user thinks it should based upon the selected items.

Changed the default colors to black. Otherwise white is set, and a blink effect with a single color would cause a blinking color/white rather than blinking of the color. I believe this also matches WLED default for 2nd, 3rd colors.

updated the readme to include changes/settings added.

Checklist

If your change touches anything under src or the README.md file these items must be done:

neilenns commented 3 years ago

Thanks for opening the pull request! Unfortunately I can't accept it in its current form. Here are some things to consider when opening new pull requests these proposed changes:

  1. This PR contains three separate changes: segment support, debug support, and changing the default LED colours. These need to be in separate PRs linked to separate issues.
  2. The PR fails build validation (linting failure).
  3. There should be information in the PR comments about how upgrades from existing installations are handled. Are these breaking changes? Have you tested what happens when an existing install applies the new version?
  4. The changelog needs to update to include a description of the changes. You can put them under an "## Unreleased" header and I'll take care of updating the version info later.

When you open new PRs for the three separate changes don't worry about updating the version number. That gets taken care of when I push out new releases on my end.

I suggest doing the PRs in this order:

  1. Debug support
  2. Updating the default LED colours
  3. Segment selection
marc-gist commented 3 years ago

Thank you for the feedback! its is valuable and I do appreciate it; however, I'm not a professional as i'm sure you can tell. I don't have time to redo these separately one at a time and fulfill the nuances you are talking about (way too time consuming for the simple changes I needed). I total understand the need for your method in a professional environment/program and wanting to keep your code as clean as you have created it (it's a great node!).

I thought to save you some time by sharing, feel free to use my changes if you wish and do your own separate "releases" for each item.

If I find myself with extra time, I will do so. In the meantime:

I would suggest that you create a guide for those who wish to contribute indicating the rules and guidelines you want followed so that the process is streamlined and you don't have to repeat yourself to "amatures" like me :)

Again, thank you for your time and this wonderful program!