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

Clean init process to prevent exceptions #834

Closed
wants to merge 1 commit into from

Conversation

dehnhardt
Copy link
Contributor

@dehnhardt dehnhardt commented Oct 24, 2023

As suggested by Robin I have moved nearly all initialization to "start_using_device" and remove all connections in "stop_using_device".
In addition I prevent updates when not connected. (And did some proper formatting...)

@x42
Copy link
Member

x42 commented Oct 24, 2023

Rebased and merged as 8.1-21 94fec7f

@x42 x42 closed this Oct 24, 2023
@x42
Copy link
Member

x42 commented Oct 25, 2023

Does this work for you?

We have a report from Harrison that the device does not respond anymore with this patch applied.
Both ports connected, so the surface should be enabled:
image

Any idea what we could check?

@x42 x42 reopened this Oct 25, 2023
@dehnhardt
Copy link
Contributor Author

I can't explain that right now. I developed it with the Mixbus code and then copied it to Ardour and tested it there with multiple connects and disconnects.
Since I'm not at home at the moment, I can't get to the bottom of the problem because I don't have the device here.
If anyone has time, the writeouts with "./ardev -D console1,midisurface" might be helpful. Otherwise, I think it's easiest to revert the commit for now, since I won't get around to fixing it until 2 November.

@x42
Copy link
Member

x42 commented Oct 26, 2023

This is on macOS, and the user does not compile Mixbus, so it was not trivial to get debug output.

The actual issue: Moving a Fader on Mixbus, updates the console1, but moving a knob on console 1surface does not update Mixbus. -- This apparently worked last week.

DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::Console1: stripable_selection_changed 
DEBUG::Console1: set_current_stripable 
DEBUG::Console1: current_stripable found:  
DEBUG::Console1: current_stripable 0 - 1025
DEBUG::Console1: current_stripable: rid 0, bank 0, index 0 
DEBUG::Console1: map_shift()
DEBUG::Console1: map_select())
DEBUG::Console1: map_phase 
DEBUG::Console1: map_recenable()
DEBUG::Console1: map_solo()
DEBUG::Console1: map_gate_scf() - shift: 0
DEBUG::Console1: ****value from comp-type 0
DEBUG::Console1: Console1::map_mute ...
DEBUG::Console1: Console1::map_mute stop blinking
DEBUG::Console1: map_select())
DEBUG::Console1: map_phase 
DEBUG::Console1: map_recenable()
DEBUG::Console1: map_solo()
DEBUG::Console1: map_gate_scf() - shift: 0
DEBUG::Console1: ****value from comp-type 0
DEBUG::Console1: Console1::map_mute ...
DEBUG::Console1: Console1::map_mute stop blinking
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in
DEBUG::MIDISurface: something happened on  Console1 in
DEBUG::MIDISurface: data available on Console1 in

@dehnhardt
Copy link
Contributor Author

Moving a Fader on Mixbus, updates the console1, but moving a knob on console 1surface does not update Mixbus. -- This apparently worked last week.

This is even more confusing - I don't see that I have changed anything in retrieveing events from the device.
I'm really sorry for the problems here.
I fear, reverting the change will be the best for the time being.

@x42
Copy link
Member

x42 commented Oct 26, 2023

After reverting 94fec7f
the user reports "Volume and Pan now works - But none of the other elements work"

There is also no constant stream of "something happened" messages any more, looks like messages are handled again:
image

--

I'll do another build with after reverting 99e2546.
I suppose that might be able to explain things, then again, there are no "Button not found" debug messages. weird

@x42
Copy link
Member

x42 commented Oct 26, 2023

OK after reverting 99e2546 solo and mute work again.
I don't yet have an explanation..

@dehnhardt
Copy link
Contributor Author

Ok, I'm not sure if your changes were already on my system - even thoug I thought I did a rebase.
Does this mean that for now only pan, volume, solo, mute is working properly or everything?
If the latter commit was the problem it might be good to reapply my last changes.

pauldavisthefirst pushed a commit that referenced this pull request Oct 26, 2023
This reverts commit 94fec7f
except whitespace and intent changes as per discussion on
#834
@x42
Copy link
Member

x42 commented Oct 26, 2023

It seems that f34ce17 prevents any Incoming from being processed.

It seems the issue is caused by calling MIDISurface::ports_acquire too early, at a point before BaseUI::run() creates the event loop.

When starting the ctrl surface there is a message

GLib-CRITICAL **: 21:51:43.236: g_main_loop_get_context: assertion 'loop != NULL' failed

coming from

#3  0x00007ffff3221119 in Glib::MainLoop::get_context() () at /lib/x86_64-linux-gnu/libglibmm-2.4.so.1
#4  0x00007fffe59296b5 in MIDISurface::ports_acquire() (this=0x61d002135480) at ../libs/ctrl-interface/midi_surface/midi_surface.cc:132
#5  0x00007fffe5927537 in MIDISurface::port_setup() (this=0x61d002135480) at ../libs/ctrl-interface/midi_surface/midi_surface.cc:58
#6  0x00007fffe2b3789d in ArdourSurface::Console1::Console1(ARDOUR::Session&) (this=0x61d002135480, s=...) at ../libs/surfaces/console1/console1.cc:59
#7  0x00007fffe2b349da in new_console1(ARDOUR::Session*) (s=0x6260000d5100) at ../libs/surfaces/console1/console1_interface.cc:33

@dehnhardt
Copy link
Contributor Author

Okay, the problem is becoming quite widespread. The error should then occur with many control surfaces, right? I copied the whole port initialization stuff from other implementations.

@pauldavisthefirst
Copy link
Contributor

It wasn't copied correctly.

Other surfaces call (one way or another) BaseUI::run() before doing port setup.

@dehnhardt
Copy link
Contributor Author

@pauldavisthefirst Maybe you can shed some more light on this. I call BaseUI::run() in the 'setActive' method. This is exactly how it is done e.g in faderport8.cc, cc121.cc...
What do I wrong here?

@x42
Copy link
Member

x42 commented Oct 27, 2023

Faderport8, does not use not use MIDISurface as base-class

@x42
Copy link
Member

x42 commented Oct 27, 2023

I have already cleaned up the code, please pull first before making further changes.

@dehnhardt
Copy link
Contributor Author

Sorry, I did not expect one of tis files being changed outside this PR. I have removed the last commit and rebased. Still, I would like to test it when I'm back home next thursday before reapplying.

libs/surfaces/console1/wscript Outdated Show resolved Hide resolved
libs/surfaces/console1/console1.cc Outdated Show resolved Hide resolved
libs/surfaces/console1/console1.cc Show resolved Hide resolved
libs/surfaces/console1/console1.cc Outdated Show resolved Hide resolved
libs/surfaces/console1/console1.cc Show resolved Hide resolved
libs/surfaces/console1/console1.cc Show resolved Hide resolved
libs/surfaces/console1/console1.cc Outdated Show resolved Hide resolved
libs/surfaces/console1/console1.cc Show resolved Hide resolved
libs/surfaces/console1/console1.cc Show resolved Hide resolved
@x42
Copy link
Member

x42 commented Oct 28, 2023

My GTKMM removal from the wscript indeed broke windows builds.
So I've merged this and made the two changes to get them back.

Thanks for your hard work!

@x42 x42 closed this Oct 28, 2023
@dehnhardt
Copy link
Contributor Author

Many thanks for your ongoing support Robin and Paul!

@dehnhardt
Copy link
Contributor Author

Sorry for commenting here again: Nathan fro Harrison contacted me again and it still seems not to be working on Apple. He tried on the Ardour nightly build 31st of October and had the same problems. From Ardour to Console1 it works, from Console 1 to Ardour it does not work.
I always work with Linux and there it works with: The last Ardour Version, the last GIT version from Ardour, the last Mixbus version and the last GIT version from Mixbus.
I do not have the chance of testing it with an Apple computer but we know that it worked with Mixbus 9.1, becaus Nathan made a video about it...
What can be wrong still?

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