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

feat: groundwork for grid size #2499

Merged
merged 58 commits into from
Jul 15, 2023
Merged

feat: groundwork for grid size #2499

merged 58 commits into from
Jul 15, 2023

Conversation

Julusian
Copy link
Member

This lays the groundwork for being able to change the grid size, with the following key changes:

  • Rework controlIds to be generated as bank:<uuid> instead of bank:<page>:<bank>
  • At places where we need a bank number, that has been replaced with a coordinate string (eg 0x0)
  • The PagesController now contains a mapping of what controlIds are at what coordinates.
  • Rework surfaces to operate in xy coordinate instead of the bank number
  • Internal actions have been updated to use coordintes instead, maintaining backwards compatibility
  • Modules are no longer informed of the page & bank their actions & feedbacks are on, both values will now be zero. These properties always been marked as deprecated in the new module api and are only there for the legacy module wrapper.

This is a very sprawling change, and it is likely to have broken things in the process.
With this change, there should be no technical reason for being limited to 8x4.

All of the protocols (including cloud) are still using the old bank numbers, further changes will be needed to allow for clients to use coordinates instead.

Julusian and others added 5 commits July 3, 2023 22:53
# Conflicts:
#	lib/Surface/Handler.js
#	lib/Surface/IP/Satellite.js
# Conflicts:
#	lib/Surface/Handler.js
#	lib/Surface/USB/LoupedeckCt.js
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Julusian
❌ dnmeid
You have signed the CLA already but the status is still pending? Let us recheck it.

@dnmeid
Copy link
Member

dnmeid commented Jul 9, 2023

The Loupedeck CT is working now with one exception: when a button gets redrawn, it will only update buttons on the upper half and the first button on the lower half. On page change, everything is drawn correctly.

I've seen that the location row and column sometimes is a number and sometimes is a string. Maybe that's the problem.
Also I'm really often am confused by the order x,y. Row,column feels much more natural when 0,0 is in the top left.

@Julusian
Copy link
Member Author

Julusian commented Jul 9, 2023

Thanks for fixing up the CT implementation

I've seen that the location row and column sometimes is a number and sometimes is a string. Maybe that's the problem.

Yeah, Im not surprised. This is an issue we already have with the page & bank. Sounds solvable, but they could come from anywhere...

Also I'm really often am confused by the order x,y. Row,column feels much more natural when 0,0 is in the top left.

Yeah, I dont feel strongly that it shouldnt change. Trying to avoid doing too much at once, so now that the way I was handling 'locations' has changed, I think this can be tackled. Im not sure if it should change when they are two parameters in the code (still referred to as x&y), or just in user facing places?

@dnmeid
Copy link
Member

dnmeid commented Jul 9, 2023

Ok, the number fix also fixed some of the CT problems (=== comparisons now work).
There are still pagenumbers as strings coming from webUI.

Row,column feels much more natural when 0,0 is in the top left.

Yeah, I dont feel strongly that it shouldnt change.

I think row,column will be definitely better. Internal xy-stuff can be left xy, but I have allready found off by one errors.

@Julusian
Copy link
Member Author

Julusian commented Jul 9, 2023

I think row,column will be definitely better. Internal xy-stuff can be left xy, but I have allready found off by one errors.

ok, I have swapped them everywhere that I think needs it.

Other than some additional testing, I think this is back to being ready

@Julusian
Copy link
Member Author

@dnmeid @krocheck let me know if you want me to hold off on merging this for a bit. Otherwise I shall merge it in a couple of days

@dnmeid
Copy link
Member

dnmeid commented Jul 11, 2023

My only concern at the moment is, that everytime I switch back from this branch, Companion won't start because of a too new database. I thought it would just ignore newer database formats and work with the old one.

@Julusian
Copy link
Member Author

My only concern at the moment is, that everytime I switch back from this branch, Companion won't start because of a too new database. I thought it would just ignore newer database formats and work with the old one.

That will happen going between this and develop, but it doesn't happen between this and beta

@Julusian Julusian merged commit 11239c7 into develop Jul 15, 2023
5 of 9 checks passed
@Julusian Julusian deleted the feat/grid-size branch July 15, 2023 19:41
@Julusian Julusian mentioned this pull request Jul 25, 2023
12 tasks
@Julusian Julusian added this to the v3.2 milestone Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants