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

Refactor/intermediate controls #28

Merged
merged 26 commits into from
Aug 25, 2023

Conversation

astingen
Copy link
Contributor

@astingen astingen commented Jul 27, 2023

This is a pretty major refactor.

When named controls or components are instantiated, the module lazy loads them from the core. The core looks up the name, and returns the control/component intermediary if it exists already. If it doesn't exist, it creates it, adds it to the collection, and returns it. Multiple modules using the same control or component all receive the same intermediary.

Modules then subscribe to the event on the intermediary. When feedback is received, the control/component intermediary is looked up by name in the collection (using a dictionary with O(1) lookup time). The feedback event on the intermediary is fired, and all subscribed modules receive the update and act accordingly.

Controls on Named Components work in a similar manor, and also have their own intermediary / lazy load process.

Many of the control names are now generated by the ControlNameUtils static class, to reduce re-use of hard-coded strings.

I added QsysMatrixMixerOutputAllCrosspoints - this is to allow for mute/unmute of all the inputs of a specified output on a matrix mixer.

There are a few things that still need to be cleaned up, eventually (but I'm not going to do now):

  1. A few named components don't use the intermediaries for feedback. See QsysCamera, QsysPotsController, and QsysRoomCombiner.
  2. There are a lot of places that still use hard-coded strings in the modules for the control names - these should be moved to ControlNameUtils.

@astingen astingen marked this pull request as draft July 27, 2023 20:54
@astingen astingen marked this pull request as ready for review July 31, 2023 19:38
@astingen
Copy link
Contributor Author

This is as far as I'm planning to take this branch at the moment, unless there are specific things you want changed ahead of a merge.

@MatKlucznyk
Copy link
Owner

This is as far as I'm planning to take this branch at the moment, unless there are specific things you want changed ahead of a merge.

Thank you for this! I've had a chance to quickly review. As soon as I can, I will fully review but so far looks amazing.

@MatKlucznyk MatKlucznyk self-requested a review August 4, 2023 11:42
Copy link
Owner

@MatKlucznyk MatKlucznyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of modules that are currently not working.

Qsys Matrix Mixer Output All Crosspoints
Qsys Room Combiner
Qsys Snapshot Controller
Qsys Camera

It looks like most of them are related to controls that require a range ie. Total snapshots

Let me know if they are working for you. If not then hopefully we can sort that out because you've done what I have wanted to do for a while :)

@PeterScream
Copy link

Oh dear that's something I desperately need ...
shall I use prev builds or sponsor a big crate of beer for someone? :))

@MatKlucznyk
Copy link
Owner

Oh dear that's something I desperately need ...
shall I use prev builds or sponsor a big crate of beer for someone? :))

You can continue to use the master branch, I haven’t merged this branch yet.

@astingen
Copy link
Contributor Author

astingen commented Aug 9, 2023

There are a couple of modules that are currently not working.

Qsys Matrix Mixer Output All Crosspoints
Qsys Room Combiner
Qsys Snapshot Controller
Qsys Camera

I'll check it out tomorrow and let you know

@PeterScream
Copy link

PeterScream commented Aug 10, 2023

Many thanks,
just found out that need snapshots.

@astingen
Copy link
Contributor Author

Many thanks,
just found out that need snapshots.

As Mat said, you can use the release version for now. Unless you need more than 8 snapshots in the same bank, or have lots of controls you're using (for reference, I've got 7,000+, which prompted this refactor), you will be fine with current release.

@PeterScream
Copy link

definetly less than 8, thats great, many thanks for a promt reply you guys are awesome

@astingen
Copy link
Contributor Author

There are a couple of modules that are currently not working.

Qsys Matrix Mixer Output All Crosspoints
Qsys Room Combiner
Qsys Snapshot Controller
Qsys Camera

Ok, updated. It was a null ref exception being thrown in a OnCoreAdded event handler, which prevented the subsequent handlers from being called.

@astingen
Copy link
Contributor Author

Related: Is your TCP client library public? I'm getting stuff printed to the console every time it sends something:

TCP_Client ID =main Sending command

@MatKlucznyk MatKlucznyk merged commit 905c27e into MatKlucznyk:master Aug 25, 2023
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.

3 participants