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

Refact: Separate LED selection and state control #957

Merged
merged 1 commit into from
Oct 26, 2022

Conversation

AsuraZeng
Copy link
Contributor

Separation of class LED selection and state control. The label of the LED corresponds to the silk screen of the iot2050.

There is a problem with the state switching of the old version, This refact would also fix it.

Signed-off-by: chao zeng chao.zeng@siemens.com

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 20, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: AsuraZeng / name: Chao Zeng (11d119f)

@jan-kiszka
Copy link
Contributor

Looks good to me on first sight but can you be more precise in the description, namely:

  • What is the benefit of the new structure?
  • How was that problem visible/reproducible with the old version?

@jan-kiszka
Copy link
Contributor

Oh, there might be the desire of reviewers who lack context to have a subject line that sets its to hardware/intel and the IOT2050 device.

@AsuraZeng
Copy link
Contributor Author

Looks good to me on first sight but can you be more precise in the description, namely:

* What is the benefit of the new structure?

* How was that problem visible/reproducible with the old version?

Has already polished the commit message.

Separation of class LED selection and state control.
The label of the LED corresponds to the silk screen of the iot2050.

Before we use one list to enumerate all the led and its state.
like: User1 Led Red User1 Led Green User1 Led Orange
      User2 Led Red User2 Led Green User2 Led Orange
if we need to add or delete the LED, we should change this list
After Separation of class LED selection and state control
one is Led selection: USER1 USER2
the other is status selection: Green Red Orange

For this structure, if the led or status changes, We don't need
to enumerate all the states.

Also there is a problem with the old version:
When we use one node to control led show green, then we use another node to control
led show red. The result we expect is led red, but the actually result is led orange.
This is caused the previous green do not turn off. State change is wrong.
This refact would also fix it.

Signed-off-by: chao zeng <chao.zeng@siemens.com>
@dceejay
Copy link
Member

dceejay commented Oct 21, 2022

please comment when this has been tested and is ready to merge.

@AsuraZeng
Copy link
Contributor Author

please comment when this has been tested and is ready to merge.

hi
sorry for the late response.
I have fully tested this modification and passed the test.

@BaochengSu
Copy link

Looks good to me.

@dceejay dceejay merged commit 3ca4456 into node-red:master Oct 26, 2022
@dceejay
Copy link
Member

dceejay commented Oct 26, 2022

Thanks - published as v0.3.0

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.

4 participants