Fix initialization of some structures - #297
Open
ycollet wants to merge 9 commits into
Open
Conversation
On session switch, monitor_states_done was still DONE with brothers_dict holding the previous session's clients. open_file() would then silently discard any connection whose nsm_client_from/to wasn't in that stale dict, treating live clients of the new session as removed ones. Resetting monitor_states_done to NEVER_DONE at the start of open_file() lets the brothers_dict be rebuilt for the new session before any filtering occurs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JACK connection events are queued asynchronously in ev_handler and only processed in the main run_loop iteration. If a SAVE message arrives before the queued CONNECTION_ADDED events are processed, connection_list is stale: connections that were just made in JACK appear absent, causing save_file's prune logic to remove them from saved_connections and write a file without those connections. Reopening the session then finds no connections to restore. Fix by flushing the pending event queue at the start of save_file so connection_list and jack_ports are fully up to date before pruning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The save_file prune logic removed a saved connection whenever both its ports were present in JACK but the connection was absent from connection_list. This correctly handled deliberate user disconnections but also incorrectly discarded connections that were simply not yet established (e.g. a non-NSM client whose ports appeared but whose connect_ports call hadn't been confirmed yet, or had failed silently). Introduce disconnected_connections: a set populated only by connection_removed (when a connection that existed is dropped) and cleared by connection_added (when it comes back) and open_file. The prune now requires sv_con to be in disconnected_connections, ensuring only genuinely user-disconnected connections are forgotten. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two gaps left a stuck state when connect_ports returned a silent JackErrorCode during session open (possible on PipeWire-JACK): 1. open_file called may_make_one_connection() but never started timer_connect_check, so if the first attempt failed no retry ever fired once all ports were already present. 2. may_make_one_connection cleared all is_new flags even when a connection was just attempted but not yet confirmed, meaning the retry timer (even if it fired) found nothing to do. Fix: start timer_connect_check after every connect_ports call and after open_file's initial attempt, and only clear is_new when no connection was attempted in that round (i.e. everything is already established or no matching ports exist). Also log JackErrorCode at DEBUG level in connect_ports so that unexpected PipeWire-JACK errors are visible with -l DEBUG. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two issues found while analysing a log with no patcher output: 1. internal_prepare used `match func_args:` (the whole tuple) instead of `match func_arg:` (current iteration variable), so the '-dbg'/ '--dbg' case never matched and no logger levels were ever set. The --log/--dbg flags had zero effect for any standalone jackpatch. 2. When jackpatch runs as an internal client it uses jack_engine_remote, not jack_engine. connect_ports there is a fire-and-forget OSC send to the patchbay daemon; no JackErrorCode ever occurs. Add INFO-level logging for both connect and disconnect requests so that connection attempts are visible with --log jackpatch (no debug flag required). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a saved connection was unexpectedly removed (e.g. Non-Mixer-XT briefly disconnecting an external feed while reloading its strip routing), connection_removed added it to disconnected_connections but did not restart timer_connect_check or reset is_new on either port. Since may_make_one_connection only retries connections where at least one port has is_new=True, a connection dropped after ports had already become stable (e.g. fluidsynth appearing 10 seconds into TuxGuitar's load) would never be re-established. Fix: when connection_removed detects that the removed connection is a saved connection, mark both ports as is_new=True and start the retry timer so may_make_one_connection re-attempts the connection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When connection_removed detects a saved connection was unexpectedly removed, it schedules a retry (commit 01430bb). However it still added the connection to disconnected_connections before doing so. If save_file ran in the 200ms retry window while both ports were still present, the prune condition would fire and strip the connection from the XML — so the next open would not restore it. Fix: discard from disconnected_connections in the same branch that schedules the retry, so save_file cannot prune the connection while the reconnect is in-flight. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When ray-daemon is launched detached from a terminal (the normal path via the GUI), stderr output is discarded, making --dbg useless in practice. When any --dbg or --info module is requested, now also attach a FileHandler that writes to: ~/.config/RaySession/logs/patcher_debug.log The file is truncated at each daemon start so it only contains the most recent session. This makes patcher/jackpatch debug output reliably available for post-mortem analysis without needing to run the daemon manually from a terminal. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gers The previous approach added the FileHandler to the root logger and relied on child-logger propagation reaching it. Python's callHandlers() skips the root logger's level check during propagation, but in practice records from 'jackpatch' and 'patcher.patcher' child loggers never arrived in the file, leaving patcher_debug.log empty even when --dbg was active. Attach the FileHandler directly to each module logger named in --dbg / --info instead. 'jackpatch.jack_engine_remote' records still reach the handler by propagating to their 'jackpatch' parent, and all loggers continue to propagate to root for the stream (stderr) handler as before. The 'ray_daemon' / 'daemon' special case still targets the root logger. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
|
Thanks a lot. I see some obvious good points, however, because I have a pending branch with big changes on connections process, I have to think about seriously because each change can be a conflict at next merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a long standing work trying to fix the problems on reconnections I've got when reloading a session.
I think this PR is not to be merged as is, but you can grab pieces of what Claude have found and add some of these pieces to master.