Model View Controller & Publish Subscriber design pattern

I have studied the source code of Zynthian. What surprises me is that the Zynthian GUI is handling the MIDI events. I expected a model in the Zynthian software. The values in that model are changed based on the midi input and the GUI is updated with some sort of publish-subscriber pattern.

Currently I catch the MSB message in the MIDI cc methodes (midi_control_change). When I set a bank and reload the presets in the file, the GUI is not updated, unless in the control screen.

Hi jan.

The current implementation of zynthian has grown way beyond what it was initially intented to do, and this means it’s a bit monolithic, so yes, you are correct, the gui is deeply intertwined with everything else.

There has been discussion (search API in the forum) about abstracting away the functional and the GUI parts of the code via an API, pub-sub seems like a good proposal as well, but, as of right now, nobody has had the time to code it and make such substantial changes to the codebase, and, therefore no architecture efforts have been fully established with that goal in mind.

I think there is a real benefit to such solution and would be eager to collaborate with either the architecture or the implementation (or both).

Honestly, my first task in this regard would be to refactor the whole code to PEP8 with a tool like black · PyPI and to eliminate/merge duplicated locations/folders/plugins/configs.
basically, some housekeeping to make sure things are organized before getting down to the nitty-gritty

Ok. That clarifies the current design with it’s limitation. I was thinking to do the refactoring in a number of steps. First step is moving the MIDI processing to a separate Zynthian_model class that contains the layers and the engines. In that case the Zynthian_gui (classes) can (1) pub/sub to a dirty variable that is changed when a var in the model changes triggering renewal of the data on the visible screen or (2) pub/sub more specific model variables / gui items. First option is more simplified as first step, but has more overhead with every change, the latter has more overhead with programming and could be used in case option first turns out too inefficient. Guess we could do some MVP.

With the current design I was thinking to trigger the method of the visible screen to renew the content, but need to reverse engineer that part first.

Hi @guys!

It’s true that current code is far from perfect and UI and core are not totally separated. And i think @Pastitas explanation is quite accurate, but …

It’s not true that MIDI is being processed by the GUI classes. MIDI is processed by ZynMidiRouter, that is a core part and it’s written in C. Engines are connected to ZynMidiRouter using jackd.

Of course, MIDI events are captured on the python side for several reasons:

  • Monitoring (the little “m” on the status bar)
  • UI actions, like:
    • Master-channel actions => CUIA, loading snapshots, etc.
    • Program Change / Bank => ZS3, loading presets
    • MIDI learning

From my point of view, the worst code parts are located on:

  • zyncoder library. It should be refactored
  • zynthian_gui_controller class is a mess, but it’s quite complex and works nicely. It should be refactored with the zyncoder!!
  • zynthian_gui_layers class is a total mess and should be refactored and splitted. A new “zynthian_chain” class should be implemented.
  • zynthian_gui class is overloaded. It should be refactored and splitted. A new “zynthian_midi” class should be implemented.

And many more, but regarding code, i think this would be a good starting point :wink:

Best regards!

Too little consensus in my view…

For now I have omitted the lack of gui updates in the preset and bank screen by removing a conditional statement. It results in changing the gui to the control screen when an instrument is selected via the midi program control command. The control screen is updated and when changing back the other screens too. Not an elegant, nevertheless effective solution. Phew next…

This is a open source project, developed in people’s free time and there’s obviously not a board of directors or regular meetings (zynclub is more of a party and a place to have some beer while talking about ideas) so yeah, if the refactor is going to take place I’d find creating a work group and having some meetings would be the way to go, and focus our efforts in that way. This particularly should be decided by @riban, who I would call lead developer, and @jofemodo, proyect leader, CTO, CEO and developer :wink:

I do believe that this change could bring a huge new level of polish to the zynthian, making debugging, reliability testing and even community contributions much easier to integrate. This, of course, is a huge undertaking and I would call for the more senior developers to raise concerns or caveats that they’ve found.

Straight away some questions come to my mind:

  • Is pub/sub fast enough? I find for anything music or sound related a model where packages or messages can get lost a much better solution, I see pub/sub more suited for configurations or something along those lines, and an API based solution better for midi/sound events. I’d rather lose a midi message then have latency.

  • Is a major change in the current architecture even necessary? Maybe the GUI code that does the heavy lifting could be moved into zyntcoder library and then setup that famous API so that the GUI code does calls to that. This would not be so far from what we have now (I think this or something like this was proposed to me by @riban in fact) and might be doable.

  • Can we use pep8 going forward? Please? (It makes code much more readable and is the current standard, there’s no drawbacks and I can do this myself as soon as I get the green light)

  • I’m also able to take on the task of cleaning up unused/duplicated folders and making everything a bit tidier as soon as I’m told it’s fine to do. (If I do end up doing this, a lot of questions will be going your way @riban and @jofemodo)

1 Like