feat: add OpenRC support for bluetooth service management#1982
Open
lannuttia wants to merge 45 commits into
Open
feat: add OpenRC support for bluetooth service management#1982lannuttia wants to merge 45 commits into
lannuttia wants to merge 45 commits into
Conversation
Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
Refactor systemd module to support generic service queries via new is_service_enabled and is_service_active functions. The existing bluetooth-specific functions now delegate to these generic helpers. Coding-Agent: Claude Code Model: claude-sonnet-4-5
Add service_manager field to Page struct to enable dependency injection of service management implementations. This allows testing with mock service managers while using real service managers in production. - Add service_manager field to Page struct with Box<dyn ServiceManager> - Convert Default derive to explicit impl with SystemDServiceManager - Add with_service_manager constructor for tests - Add test verifying Page can be constructed with MockServiceManager Coding-Agent: Claude Code Model: claude-sonnet-4-5
Update the DBusConnect message handler to query the injected service_manager instead of calling systemd functions directly. This enables proper dependency injection and testing with mock service managers. - Replace systemd::is_bluetooth_active() with service_manager.is_active() - Replace systemd::is_bluetooth_enabled() with service_manager.is_enabled() - Add test demonstrating service manager can return different statuses Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Modified ServiceManager trait to return Pin<Box<dyn Future<Output = ()> + Send>> from activate()
- Updated MockServiceManager to return async {} wrapped in Box::pin
- Updated SystemDServiceManager to call systemd::activate_bluetooth() wrapped in Box::pin
- Added test to verify activate() returns an awaitable Future
- Updated existing tests to use async/await with activate()
Coding-Agent: Claude Code
Model: claude-sonnet-4-5
- Modified ServiceManager trait to return Pin<Box<dyn Future<Output = ()> + Send>> from enable()
- Updated MockServiceManager to return async {} wrapped in Box::pin
- Updated SystemDServiceManager to call systemd::enable_bluetooth() wrapped in Box::pin
- Added test to verify enable() returns an awaitable Future
- Updated existing tests to use async/await with enable()
Coding-Agent: Claude Code
Model: claude-sonnet-4-5
Migrate the ServiceActivate message handler to use the ServiceManager trait's activate() method instead of directly calling systemd functions. This enables support for different service managers (SystemD, OpenRC). Add test to verify that handling ServiceActivate calls the service manager's activate() method. Coding-Agent: Claude Code Model: claude-sonnet-4-5
Migrate the ServiceEnable message handler to use the ServiceManager trait's enable() method instead of directly calling systemd functions. This enables support for different service managers (SystemD, OpenRC). Add test to verify that handling ServiceEnable calls the service manager's enable() method. Coding-Agent: Claude Code Model: claude-sonnet-4-5
…in production) Coding-Agent: Claude Code Model: claude-sonnet-4-5
…detection Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Test DBusConnect queries service manager for enabled/active states - Test service manager correctly reports enabled=true, active=true - Test service manager correctly reports enabled=true, active=false Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Move ServiceManager trait, MockServiceManager, and SystemDServiceManager to new service_manager.rs module - Update bluetooth page to import from service_manager module - Migrate existing service manager tests to new module Coding-Agent: Claude Code Model: claude-sonnet-4-5
Removed tests that use tracking/spy pattern to verify methods were called. These tests coupled to implementation details rather than observable behavior. Kept only behavior-focused tests that verify actual state changes. Tests removed: - test_page_queries_service_manager_for_bluetooth_status (redundant) - test_service_activate_message_calls_service_manager_activate (spy) - test_service_enable_message_calls_service_manager_enable (spy) Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Add systemd and openrc cargo features (systemd enabled by default) - Implement OpenRcServiceManager with OpenRC command support: - is_enabled: checks rc-update show output - is_active: uses rc-service status - activate: uses rc-service start - enable: uses rc-update add + rc-service start - Add test for OpenRC implementation (conditional on feature) - Clean up unused imports in bluetooth tests OpenRC implementation uses: - rc-update show: check if service is enabled in runlevels - rc-service status: check if service is currently running - rc-service start: start a service - rc-update add: enable service in default runlevel Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Add conditional compilation attributes to SystemDServiceManager (systemd feature) - Remove embedded systemd module, make implementation standalone - Update SystemDServiceManager to use service parameter (generic like OpenRC) - Update create_default_service_manager() to support feature-based selection: 1. Test mode: MockServiceManager 2. openrc feature: OpenRcServiceManager 3. systemd feature (default): SystemDServiceManager - Make systemd test conditional on feature - Remove hardcoded bluetooth service names from systemd implementation Changes: - SystemD activate/enable now use the service parameter instead of hardcoded 'bluetooth' - Both implementations now follow the same pattern with async service operations - Eliminated the systemd helper module in favor of inline implementation Coding-Agent: Claude Code Model: claude-sonnet-4-5
Replace compile-time feature selection with runtime detection: Detection strategy: 1. Check for systemd: Run 'systemctl --version' 2. Check for OpenRC: Run 'rc-service --version' 3. Return descriptive error if no supported service manager found Benefits: - Single binary works on both systemd and OpenRC systems - No need to compile separate versions for different init systems - Graceful fallback with error logging if detection fails Implementation: - detect_service_manager(): Attempts to detect running service manager - create_default_service_manager(): Uses detection in production, mock in tests - Fallback to no-op MockServiceManager if detection fails (prevents crash) - Conditional compilation still used to only include code for enabled features Error handling: - Descriptive error messages indicating which service managers were attempted - Graceful degradation - app continues running but service management disabled - Error logged via tracing for debugging Coding-Agent: Claude Code Model: claude-sonnet-4-5
Replace command-based detection with runtime directory checks: SystemD detection: - Check for /run/systemd/system directory - This only exists when systemd is actively running as service manager - More reliable than checking if systemctl command exists OpenRC detection: - Check for /run/openrc directory - This exists when OpenRC is actively managing services - Works correctly regardless of init system (sysvinit, runit, etc.) - Important: OpenRC is a service manager, not an init system (e.g., Gentoo uses sysvinit as PID 1 but OpenRC for service management) Benefits: - Detects RUNNING service managers, not just installed ones - Handles cases where multiple service managers are installed - More accurate than checking for command existence - No forking processes just for detection - Added debug logging for detection results Error messages: - Updated to mention runtime indicators being checked - Clearer explanation of what was attempted Coding-Agent: Claude Code Model: claude-sonnet-4-5
Make error messages more accurate based on enabled features: When no features enabled: - Clear message: "No service manager features enabled at compile time" - Suggests enabling systemd or openrc feature When features enabled but no service manager detected: - Shows exactly what was checked with paths - Example: "Checked for: systemd (/run/systemd/system), openrc (/run/openrc)" - Clearer than previous "Attempted" wording Benefits: - More actionable error messages - Shows specific runtime indicators that were checked - Distinguishes between build-time and runtime issues - Helps users understand why detection failed Coding-Agent: Claude Code Model: claude-sonnet-4-5
Problem: MockServiceManager is only available in test mode, but we were trying to use it as a fallback in production code. Solution: Create a separate NoOpServiceManager for production use: - Always reports services as enabled and active - No-op implementations for activate/enable - Allows the application to continue running - Service management just won't actually work Benefits: - Application doesn't crash if service manager detection fails - Clear warning logged when fallback is used - Users can still access other settings even if service management is unavailable - Graceful degradation instead of fail-fast Implementation: - NoOpServiceManager: Simple struct implementing ServiceManager trait - Used only in production when detect_service_manager() fails - Logs warning to help with debugging Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Add service_name field to all ServiceManager implementations - Remove service parameter from trait methods (is_enabled, is_active, activate, enable) - Update factory functions to accept service_name parameter - Improve variable naming: use service name instead of generic 'manager' or 'mock' - Add warning log when falling back to NoOpServiceManager - All tests passing with clearer, more intuitive API (e.g., bluetooth.is_enabled()) Coding-Agent: Claude Code Model: claude-sonnet-4-5
When the bluetooth service is enabled (will start on boot) but currently inactive (not running), the UI button says 'activate' but was incorrectly sending Message::ServiceEnable. This would work for systemd (enable --now starts the service) and OpenRC (enable runs both rc-update add and rc-service start), but it's semantically wrong and performs unnecessary operations. Now correctly sends Message::ServiceActivate, which only starts the service without modifying boot configuration. Coding-Agent: Claude Code Model: claude-sonnet-4-5
…alled Coding-Agent: Claude Code Model: claude-sonnet-4-5
…ceEnable handlers Coding-Agent: Claude Code Model: claude-sonnet-4-5
…is tested Coding-Agent: Claude Code Model: claude-sonnet-4-5
…ez unknown message When no service manager is detected at runtime (e.g., building with systemd feature on OpenRC), NoOpServiceManager is used as the fallback. Previously it reported is_installed() = true, which caused the DBusServiceUnknown handler to take the 'installed' branch and set service_is_enabled/active to true. This left bluez_service_unknown = false, so the UI showed a non-functional toggle instead of the 'bluetooth unknown' message. By returning false, the DBusServiceUnknown handler correctly falls through to the bluez_service_unknown = true path, displaying the appropriate message. Coding-Agent: Claude Code Model: claude-sonnet-4-5
lannuttia
force-pushed
the
fix/support-openrc-bluetooth-service-management
branch
from
faa321a to
c4d5b31
Compare
… matching The previous implementation used startswith() to match service names against rc-update show output, which could falsely match a similar service name (e.g. 'bluetoothd' matching a lookup for 'bluetooth'). Extracted is_service_in_runlevel_output() with exact matching (split on '|' separator, compare trimmed name) and added dedicated tests. Coding-Agent: Claude Code Model: claude-sonnet-4-5
…lable Replace unwrap() with an if-let pattern that returns early when the session bus is unavailable. This prevents test panics in headless CI environments (e.g., GitHub Actions runners without a D-Bus session). Coding-Agent: Claude Code Model: claude-sonnet-4-5
Introduce log_command_result() and run_pkexec_command() helper functions that log warnings/errors when privileged service management commands fail, instead of silently discarding the result. All four pkexec call sites (systemd activate/enable and OpenRC activate/enable) now use these helpers. Coding-Agent: Claude Code Model: claude-sonnet-4-5
Replace the early-return D-Bus skip pattern with a proper mock D-Bus connection. Uses zbus's p2p feature with UnixStream::pair() to create a fully in-memory D-Bus connection (no subprocess, no filesystem, no real session bus dependency). Both ends of the socket-pair run within the same process: a background task serves as the p2p server while the test uses the client end. The helper panics on failure — since this is an in-memory operation with no external dependencies, any failure is a bug that should fail the test loudly, not silently succeed. Enabled the 'p2p' feature on the zbus dependency (additive, no external dependencies). Coding-Agent: Claude Code Model: claude-sonnet-4-5
These two tests (test_dbus_connect_queries_service_manager_when_enabled_and_active and test_dbus_connect_handles_enabled_but_inactive_service) required creating a zbus::Connection via UnixStream pair in p2p mode, which pulled in the uuid crate as a transitive dependency. The same service-manager-querying behavior is already covered by the DBusServiceUnknown tests (test_dbus_service_unknown_with_installed_service_queries_manager et al.), so no unique coverage is lost. Coding-Agent: Claude Code Model: claude-sonnet-4-5
The p2p feature is no longer needed now that the in-memory mock D-Bus connection tests have been restructured to avoid requiring a real zbus::Connection value. This removes the uuid crate as a transitive dependency from production builds. Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
- Removed AAA (Arrange/Act/Assert) scaffolding from all tests - Removed doc comments that merely restated function names - Condensed inline comments that explained what the code does - Preserved comments explaining rationale (why exact matching, why directory-based detection, etc.) Coding-Agent: Claude Code Model: claude-sonnet-4-5
…e(Default)] on Page Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
…, remove workaround assignments in handler Coding-Agent: Claude Code Model: claude-sonnet-4-5
Contributor
Author
|
@mmstick I think this should be ready for review. |
mmstick
requested changes
May 20, 2026
Member
mmstick
left a comment
mmstick
left a comment
There was a problem hiding this comment.
Make sure debian/rules has the systemd feature added. We should also add a compiler check to ensure that one or the other is selected, but not both.
Contributor
Author
|
I would think that we would want the ability to compile in support for both SystemD and OpenRC at the same time. It should be detecting what service manager is currently in use at runtime and using that to provide the correct implementation. Technically speaking, I can't think of a reason that we'd need to only compile in one or the other. On the topic of ensuring that at least one is enabled, I don't think we'd need to do that either. If neither one of the features is enabled, then you will just get the NoOpServiceManager. Then we would just lose the automated service management and will get the error about Bluez not being installed if the Bluetooth daemon isn't running. If you think we need to make these restrictions, we can. I just want to point out that I don't think those restrictions are strictly necessary. |
Member
|
If both can be enabled, we should do that. Functionality that depends on it should require one or both. |
Coding-Agent: Claude Code Model: claude-sonnet-4-5
Coding-Agent: Claude Code Model: claude-sonnet-4-5
Contributor
Author
|
The changes that you requested should be pushed now. |
Contributor
Author
|
@mmstick the issues you mentioned should be addressed |
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.
Summary
Adds OpenRC init system support to cosmic-settings' bluetooth service management via a trait-based service manager abstraction, while maintaining full backwards compatibility with systemd-based distributions.
Fixes #1981
Changes
Core Implementation
src/service_manager.rs)ServiceManagertrait withis_enabled(),is_active(),activate(),enable(), andis_installed()methodsSystemDServiceManager: systemd implementation usingpkexec systemctlOpenRcServiceManager: OpenRC implementation usingpkexec rc-service/pkexec rc-updateNoOpServiceManager: fallback when no supported service manager is detected at runtime (returnsis_installed() = false)/run/systemd/systemfor systemd,/run/openrcfor OpenRCsrc/pages/bluetooth/mod.rs)ServiceManagertraitNoOpServiceManagerwhen service manager is unavailableFeature Flags & Build System
systemdfeature flag (included in default features, preserves existing behavior)--features openrcflagDocumentation
service_manager.rsexplaining the architectureBackwards Compatibility
This change is fully backwards compatible:
Impact
Before
On OpenRC systems:
rc-service bluetooth start)After
On OpenRC systems with
openrcfeature enabled:rc-service bluetooth status)rc-update add/rc-service start)For Distributors
systemd-based distributions
No changes needed — default build works as before:
OpenRC-based distributions (Gentoo, Alpine, etc.)
Enable the
openrcfeature:Multi-init distributions
Enable
openrcfor runtime detection of both init systems: