Improve sound server fallback and speech acknowledgement handling
Try the invoking user session followed by /run/pulse/native for sound cues, while preserving an explicit PULSE_SERVER override. Resolve the user socket from SUDO_UID, XDG_RUNTIME_DIR, or the current UID, and document the fallback and Speech Dispatcher audio configuration. Allow five seconds for Speech Dispatcher command acknowledgements so large voice catalog queries can finish without premature failures. Add regression coverage for socket selection and delayed replies. Include cpu-usage.patch as a standalone patch artifact containing Python remote registry permission and registration failure handling changes. The patch is stored without applying it to the Python sources. Validation: go test ./internal/sound ./internal/speech -count=1 passed with GOCACHE=/tmp/cthulhu-go-cache and GOPROXY=off outside the sandbox to permit local Unix sockets. source diff whitespace checks passed. The staged patch has whitespace warnings from preserved blank context lines; its contents are retained unchanged. Live audio and keyboard behavior were not tested.
This commit is contained in:
1 parent
60abcc37d9
commit
db6955f5b7
7 files changed
+177
-10
No files matched your search
@@ -74,6 +74,11 @@ server or ALSA device. The [PulseAudio](tools/configure_pulse.sh) and
|
||||
[PipeWire](tools/configure_pipewire.sh) configuration scripts are included for
|
||||
systems that need root audio access.
|
||||
|
||||
The default sound driver tries the user session first, then the system-wide
|
||||
PulseAudio-compatible socket at `/run/pulse/native`, including when provided by
|
||||
PipeWire. Set `PULSE_SERVER` to select a different socket or server explicitly.
|
||||
Speech output uses Speech Dispatcher's own audio configuration.
|
||||
|
||||
### X11 terminal
|
||||
|
||||
From an X11 terminal that supplies `WINDOWID`:
|
||||
|
||||
+123
@@ -0,0 +1,123 @@
|
||||
diff --git a/src/fenrirscreenreader/core/remoteInstanceRegistry.py b/src/fenrirscreenreader/core/remoteInstanceRegistry.py
|
||||
index 5733f3ab..07d707af 100644
|
||||
--- a/src/fenrirscreenreader/core/remoteInstanceRegistry.py
|
||||
+++ b/src/fenrirscreenreader/core/remoteInstanceRegistry.py
|
||||
@@ -12,6 +12,7 @@ import time
|
||||
|
||||
|
||||
INSTANCE_TIMEOUT_SEC = 30.0
|
||||
+REGISTRY_DIR_MODE = 0o1777
|
||||
|
||||
|
||||
def get_registry_dir():
|
||||
@@ -26,8 +27,8 @@ def get_instance_file(pid=None):
|
||||
|
||||
def write_instance(instance_data):
|
||||
registry_dir = get_registry_dir()
|
||||
- os.makedirs(registry_dir, mode=0o755, exist_ok=True)
|
||||
- os.chmod(registry_dir, 0o755)
|
||||
+ os.makedirs(registry_dir, mode=REGISTRY_DIR_MODE, exist_ok=True)
|
||||
+ os.chmod(registry_dir, REGISTRY_DIR_MODE)
|
||||
prune_stale_instances()
|
||||
instance_data["updated_at"] = time.time()
|
||||
instance_path = get_instance_file(instance_data.get("pid"))
|
||||
diff --git a/src/fenrirscreenreader/remoteDriver/unixDriver.py b/src/fenrirscreenreader/remoteDriver/unixDriver.py
|
||||
index e7411184..720671cb 100644
|
||||
--- a/src/fenrirscreenreader/remoteDriver/unixDriver.py
|
||||
+++ b/src/fenrirscreenreader/remoteDriver/unixDriver.py
|
||||
@@ -114,6 +114,17 @@ class driver(remoteDriver):
|
||||
}
|
||||
remoteInstanceRegistry.write_instance(instance_data)
|
||||
|
||||
+ def _try_register_instance(self):
|
||||
+ try:
|
||||
+ self._register_instance()
|
||||
+ return True
|
||||
+ except Exception as e:
|
||||
+ self.env["runtime"]["DebugManager"].write_debug_out(
|
||||
+ "unixDriver could not register instance: " + str(e),
|
||||
+ debug.DebugLevel.ERROR,
|
||||
+ )
|
||||
+ return False
|
||||
+
|
||||
def _socket_file_matches_socket(self, fenrir_sock, socket_file):
|
||||
try:
|
||||
if os.stat(socket_file).st_uid != os.getuid():
|
||||
@@ -328,11 +339,11 @@ class driver(remoteDriver):
|
||||
if not self.fenrirSocks:
|
||||
return
|
||||
|
||||
- self._register_instance()
|
||||
+ self._try_register_instance()
|
||||
last_register = time.time()
|
||||
while active.value:
|
||||
if time.time() - last_register > 10.0:
|
||||
- self._register_instance()
|
||||
+ self._try_register_instance()
|
||||
last_register = time.time()
|
||||
|
||||
# Check if the client is still connected and if data is available:
|
||||
diff --git a/tests/unit/test_remote_instance_registry.py b/tests/unit/test_remote_instance_registry.py
|
||||
index 7955665a..1b9aec28 100644
|
||||
--- a/tests/unit/test_remote_instance_registry.py
|
||||
+++ b/tests/unit/test_remote_instance_registry.py
|
||||
@@ -1,9 +1,24 @@
|
||||
import json
|
||||
+import stat
|
||||
import time
|
||||
|
||||
from fenrirscreenreader.core import remoteInstanceRegistry
|
||||
|
||||
|
||||
+def test_write_instance_creates_shared_sticky_registry_directory(
|
||||
+ tmp_path, monkeypatch
|
||||
+):
|
||||
+ registry_dir = tmp_path / "instances"
|
||||
+ monkeypatch.setattr(
|
||||
+ remoteInstanceRegistry, "get_registry_dir", lambda: str(registry_dir)
|
||||
+ )
|
||||
+
|
||||
+ remoteInstanceRegistry.write_instance({"pid": 456})
|
||||
+
|
||||
+ mode = stat.S_IMODE(registry_dir.stat().st_mode)
|
||||
+ assert mode == 0o1777
|
||||
+
|
||||
+
|
||||
def test_write_instance_prunes_stale_registry_files(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
remoteInstanceRegistry, "get_registry_dir", lambda: str(tmp_path)
|
||||
diff --git a/tests/unit/test_unix_remote_driver.py b/tests/unit/test_unix_remote_driver.py
|
||||
index ea95d816..325aa733 100644
|
||||
--- a/tests/unit/test_unix_remote_driver.py
|
||||
+++ b/tests/unit/test_unix_remote_driver.py
|
||||
@@ -3,6 +3,30 @@ from unittest.mock import Mock, mock_open, patch
|
||||
from fenrirscreenreader.remoteDriver import unixDriver
|
||||
|
||||
|
||||
+def test_watchdog_keeps_serving_when_instance_registration_fails(
|
||||
+ mock_environment,
|
||||
+):
|
||||
+ driver = unixDriver.driver()
|
||||
+ driver.env = mock_environment
|
||||
+ driver.fenrirSocks = [Mock()]
|
||||
+ active = Mock(value=True)
|
||||
+ event_queue = Mock()
|
||||
+
|
||||
+ def stop_after_select(*_args):
|
||||
+ active.value = False
|
||||
+ return ([], [], [])
|
||||
+
|
||||
+ with patch.object(driver, "_get_socket_candidates", return_value=[]), patch.object(
|
||||
+ driver, "_register_instance", side_effect=PermissionError("denied")
|
||||
+ ), patch(
|
||||
+ "fenrirscreenreader.remoteDriver.unixDriver.select.select",
|
||||
+ side_effect=stop_after_select,
|
||||
+ ) as select_call:
|
||||
+ driver.watch_dog(active, event_queue)
|
||||
+
|
||||
+ select_call.assert_called_once()
|
||||
+
|
||||
+
|
||||
class FakeClientSocket:
|
||||
def __init__(self, data):
|
||||
self.data = data
|
||||
@@ -29,11 +29,9 @@ import (
|
||||
|
||||
func open_pulse(s *Service, _ string) error {
|
||||
options := []pulse.ClientOption{pulse.ClientApplicationName("Fenrir sounds"), pulse.ClientTimeout(500 * time.Millisecond)}
|
||||
// Match the invoking user's session when sudo strips XDG_RUNTIME_DIR.
|
||||
// An explicit PULSE_SERVER continues to take precedence.
|
||||
if server := Server(os.Getenv("PULSE_SERVER"), os.Getenv("SUDO_UID")); server != "" {
|
||||
options = append(options, pulse.ClientServerString(server))
|
||||
}
|
||||
// Try the invoking user's session, then the system-wide service, unless
|
||||
// PULSE_SERVER explicitly selects a server.
|
||||
options = append(options, pulse.ClientServerString(Server(os.Getenv("PULSE_SERVER"), os.Getenv("SUDO_UID"))))
|
||||
client, err := pulse.NewClient(options...)
|
||||
if err != nil {
|
||||
return fmt.Errorf("connect sound server: %w", err)
|
||||
|
||||
@@ -23,6 +23,8 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"math"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
@@ -84,14 +86,20 @@ func (s *Service) Set_volume(volume float64) {
|
||||
s.volume.Store(math.Float32bits(float32(volume)))
|
||||
}
|
||||
|
||||
// Server returns PulseAudio protocol addresses in connection order. PipeWire's
|
||||
// PulseAudio service can expose the system-wide /run/pulse/native socket.
|
||||
// The client library tries each address, including after authentication errors.
|
||||
func Server(explicit, sudoUID string) string {
|
||||
if explicit != "" {
|
||||
return explicit
|
||||
}
|
||||
runtimeDir := os.Getenv("XDG_RUNTIME_DIR")
|
||||
if uid, err := strconv.ParseUint(sudoUID, 10, 32); err == nil {
|
||||
return fmt.Sprintf("unix:/run/user/%d/pulse/native", uid)
|
||||
runtimeDir = fmt.Sprintf("/run/user/%d", uid)
|
||||
} else if runtimeDir == "" {
|
||||
runtimeDir = fmt.Sprintf("/run/user/%d", os.Getuid())
|
||||
}
|
||||
return ""
|
||||
return "unix:" + filepath.Join(runtimeDir, "pulse/native") + " unix:/run/pulse/native"
|
||||
}
|
||||
|
||||
// Play only replaces a mailbox pointer. Startup has its own slot so initial
|
||||
|
||||
@@ -20,6 +20,8 @@ package sound
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"runtime"
|
||||
"slices"
|
||||
"sync"
|
||||
@@ -264,19 +266,28 @@ func TestPlayReportsUnavailableCues(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestSoundServerFollowsExplicitAndSudoSession(t *testing.T) {
|
||||
t.Setenv("XDG_RUNTIME_DIR", "/run/user/2000")
|
||||
if got := Server("unix:/tmp/example-pulse", "1000"); got != "unix:/tmp/example-pulse" {
|
||||
t.Fatal(got)
|
||||
}
|
||||
if got := Server("", "1000"); got != "unix:/run/user/1000/pulse/native" {
|
||||
if got := Server("", "1000"); got != "unix:/run/user/1000/pulse/native unix:/run/pulse/native" {
|
||||
t.Fatal(got)
|
||||
}
|
||||
for _, uid := range []string{"", "invalid", "../other"} {
|
||||
if got := Server("", uid); got != "" {
|
||||
if got := Server("", uid); got != "unix:/run/user/2000/pulse/native unix:/run/pulse/native" {
|
||||
t.Fatal(got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSoundServerWithoutRuntimeDirectoryUsesCurrentUserAndSystemFallback(t *testing.T) {
|
||||
t.Setenv("XDG_RUNTIME_DIR", "")
|
||||
want := fmt.Sprintf("unix:/run/user/%d/pulse/native unix:/run/pulse/native", os.Getuid())
|
||||
if got := Server("", ""); got != want {
|
||||
t.Fatalf("Server() = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func BenchmarkCueAdmission(b *testing.B) {
|
||||
s := &Service{clips: map[presentation.Cue]*clip{presentation.Blank: {samples: []float32{1, 1}}}}
|
||||
s.Set_volume(1)
|
||||
|
||||
@@ -34,7 +34,10 @@ import (
|
||||
"git.stormux.org/storm/fenrir/internal/diagnostic"
|
||||
)
|
||||
|
||||
const commandTimeout = 750 * time.Millisecond
|
||||
// Speech Dispatcher serializes some module queries with speech commands.
|
||||
// Large voice catalogs can delay acknowledgements beyond a fraction of a second.
|
||||
// Keep the wait bounded while allowing those queries to finish.
|
||||
const commandTimeout = 5 * time.Second
|
||||
|
||||
type Rejection struct{ Code string }
|
||||
|
||||
|
||||
@@ -25,8 +25,27 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestSpeechAcknowledgementWaitsForBusyDispatcher(t *testing.T) {
|
||||
server := new_fake_server(t, true)
|
||||
client, err := Open(Options{Socket: server.path})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer client.Close()
|
||||
result := make(chan error, 1)
|
||||
go func() { result <- client.Say("startup") }()
|
||||
wait_event(t, server.events, "say:startup")
|
||||
// Voice catalog requests can occupy the dispatcher for more than 750 ms.
|
||||
time.Sleep(time.Second)
|
||||
close(server.ack)
|
||||
if err := <-result; err != nil {
|
||||
t.Fatalf("speech failed while dispatcher was busy: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
type fakeServer struct {
|
||||
path string
|
||||
events chan string
|
||||
|
||||
Reference in new issue
Block a user