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

also sync segment name through udp #4482

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Liliputech
Copy link

This commit solves this issue : #4468

@netmindz
Copy link
Collaborator

Should this be an optional sync attribute, like the existing ones are?

@Liliputech
Copy link
Author

as of now its not optional but I can make it optional if needed.
I was not sure in which category to add this (effects, segment bounds, options?), or if I should create a new category.
The name sometime is used by the effect for the effect configuration (see GIF image branch).

@blazoncek
Copy link
Collaborator

This is a breaking change. Old devices may crash or behave incorrectly due to message containing unexpected content.
Please rework so older devices will behave correctly.

@Liliputech
Copy link
Author

This is to be tested but since the "segment packet size" is sent in the UDP packet and read by the receiver it may be backward compatible.

@netmindz
Copy link
Collaborator

I'd suggest a new checkbox called Segment Name

@Liliputech
Copy link
Author

Ok, I'll work on this and will push some new commits to reflect this change

this option is disabled by default.
When enabled, the segment name is applied when received from another
WLED instance.
@Liliputech
Copy link
Author

one new commit is available with option to enable or disable segment name sync (disabled by default like Segment Options).

@netmindz
Copy link
Collaborator

What testing have you done so far with regards to @blazoncek 's comments about compatibility testing between systems using your new packet format and without?

@Liliputech
Copy link
Author

Unfortunately didn't had the opportunity to run such tests yet. I will do ASAP
My idea of how to test this : flash one board with my branch and another board with main branch without packet format change.
then enable sync on both boards and see if everything is fine (color/segment options change).
Also see change the segment name on the board with my branch, trigger an update and see if the other board don't crash.

@netmindz
Copy link
Collaborator

Unfortunately didn't had the opportunity to run such tests yet. I will do ASAP My idea of how to test this : flash one board with my branch and another board with main branch without packet format change. then enable sync on both boards and see if everything is fine (color/segment options change). Also see change the segment name on the board with my branch, trigger an update and see if the other board don't crash.

When you mean sync on both, do you mean making one the sender and one the receiver and then doing the same the other way round? We need to ensure we cover both "forward" and "backwards" compatibility

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

Successfully merging this pull request may close these issues.

3 participants