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

Cosmetic 2D Setup change: Start at Panel 1 instead of 0 #201

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

troyhacks
Copy link
Collaborator

GUI label change only. Start at Panel 1 (instead of Panel 0) to assist with wiring things "in the real world" that don't start counting at zero. 😄

@troyhacks troyhacks merged commit 859e56a into MoonModules:mdev Dec 4, 2024
1 check passed
@dosipod
Copy link

dosipod commented Dec 4, 2024

Sure no one cares in the real world about zero being a number , why not add that to AC and while you are at it also see other areas that does not need to start at zero
image

@ewowi
Copy link

ewowi commented Dec 4, 2024

Sure no one cares in the real world about zero being a number , why not add that to AC and while you are at it also see other areas that does not need to start at zero

that’s a very swampy area (is this proper English) if we go that way, as everything starts with 0: led 0, xy=(0,0), column 0, row 0 etc
I trained myself many years to think this way, and code will keep working this way , so might be very confusing.

but maybe it will work out fine 🙂

@troyhacks
Copy link
Collaborator Author

Sure no one cares in the real world about zero being a number , why not add that to AC and while you are at it also see other areas that does not need to start at zero

that’s a very swampy area (is this proper English) if we go that way, as everything starts with 0: led 0, xy=(0,0), column 0, row 0 etc I trained myself many years to think this way, and code will keep working this way , so might be very confusing.

but maybe it will work out fine 🙂

Name me one time the 2D panel label number starting at zero was in any way a detriment.

On my end, Panel 0 corresponds to Port 1 on my Art-Net controller. When you have 20 cables and a deadline looming, a 1:1 numbering scheme is helpful.

This literally is a do-nothing patch except adding +1 to two labels in the GUI. Behind the scenes all the 2D stuff still starts at zero. 😄

@ewowi
Copy link

ewowi commented Dec 4, 2024

I can agree with this specific case but I have my doubts of looking for other stuff we can renumber

@troyhacks
Copy link
Collaborator Author

I can agree with this specific case but I have my doubts of looking for other stuff we can renumber

To be clear, nothing is "renumbered" except for the human-readable label.

@dosipod
Copy link

dosipod commented Dec 5, 2024

Yeah as the PR title is Cosmetic , it would only be the label without any change of the logic .
This is to keep the ui consistent as now you have led output and 2D page start at 1 but buttons start at 0 , if you have no interest then I will do that for buttons ( and macro page ) but only in AC

@ewowi
Copy link

ewowi commented Dec 5, 2024

It’s fine, I just don’t know if in the end it gets confusing, maybe not, I just raised it for discussion / thought

it’s okay you start buttons at 1. But then there is gpio starting at 0 still 🙂

IMG_8859

this is also interesting, start X is 0, stop X is 16 (but actually 15 as 16 is not included), maybe set startX to 1 ?

@troyhacks
Copy link
Collaborator Author

@ewowi you're not wrong in how we deal with numbers as labels in WLED. It's confusing all around.

The design pattern in use is using "programmer numbers" - which make sense to programmers. This also reduces confusion when debugging as we see labels on the screen correspond to the code.

I'm not sure there should be a complete overhaul of all the numbering schemes, but I agree the segment numbers are weird for start and stop.

My patch doesn't touch logic so it's non-breaking - but I think changing segments to "human numbers" would likely be a breaking change, and at least require people to modify presets, etc.

@ewoudwijma
Copy link
Collaborator

I'm all in for an 'architectural' decision how to deal with it. From a users perspective things should start at 1, fully agree.
Curious what @softhack007 thinks?

@softhack007
Copy link
Collaborator

softhack007 commented Dec 5, 2024

Curious what @softhack007 thinks?

Well not an easy decision. We might also change button numbering to be 1...4 instead of 0...3, but it will break lots of legacy scripting and tools (HA included). While the panel numbering is good, I'd say keep other stuff as-is when numbers start at 0.

Finally, GPIO numbering also starts at "GPIO 0" which is a perfectly valid pin. So let's talk to espressif about that 😜 (just joking).

Moving everything to "starts at 1" logic will introduce lots of new "off by one" problems in the code - arrays in C start at array[0] and this will never change.

Additionally, any leds[XY(x -1, y-1)] will cost us at least two extra instructions - which draws down performance unnecessarily.

@softhack007
Copy link
Collaborator

softhack007 commented Dec 5, 2024

I could imagine to only change the LEDs settings page and segments page to "starts at 1" logic.
We could even make it optional in UI preferences.

Bringing the numbers back to C style "starts at 0" could be done by some JS post-processing, or in WLED set.cpp/cfg.cpp/json.cpp/xml.cpp when reading out UI inputs.

This means that all non-UI code will still run as before, without needing to worry about crazy "starts at 1" user preferences.

Speaking from computer perspective 🤓: if you need one and get one, then you have one. So as a user, you'd say that "-1 +1 = 1" ?

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.

5 participants