Open
Conversation
Open
tlsim
force-pushed
the
claude/add-plan-md-file-018ndfS16yeoFxPVKX2H4pNV
branch
from
4f66d66 to
d9e7d7c
Compare
Replace hardcoded --cli flag with flexible client discovery system. Breaking Changes: - Remove --cli flag (use --client cli instead) Features: - Add client discovery module for automatic client detection - Add --client flag to specify client (e.g., --client cli, --client gfxhat) - Add --list-clients flag to display available built-in clients - Add config-based default client selection via [Client] section - Support external custom clients via module:class specification - Simplify logging configuration to use client-provided handlers Implementation: - Create src/pi_pianoteq/client/discovery.py with client discovery logic - Add get_logging_handler() abstract method to Client base class - Implement get_logging_handler() in CliClient and GfxhatClient - Update logging_config.py to accept optional handler parameter - Add CLIENT config option to pi_pianoteq.conf and Config class - Update __main__.py to use config-based client selection - Add comprehensive tests for client discovery and config Documentation: - Update docs/development.md with custom client usage examples - Update CHANGELOG.md with breaking change notice
Add explanatory note about why some clients may not be visible due to missing hardware dependencies. This clarifies for dev machine users why gfxhat isn't shown without python3-smbus installed.
Track and display clients that exist but can't be imported due to missing dependencies. This provides better visibility into what clients are available but not currently usable. Example output: cli - CLI client for Pianoteq... gfxhat - [unavailable: missing dependency: python3-smbus] Changes: - Add discover_builtin_clients_with_errors() to track import failures - Update list_clients() to show both available and unavailable clients - Add tests for error tracking functionality - Clean up error messages for better readability
Simplify and improve error handling in client discovery: 1. Remove systemd section from development.md (belongs in deployment docs) 2. Remove special case for python3-smbus - handle all errors uniformly 3. Simplify error handling logic: - Inner try/except: catches errors importing *_client.py submodule - Outer try/except: catches errors importing package itself - Both use shared _format_error_message() helper for consistency Error messages now consistently show first line of actual error, providing useful context without cluttering the output.
Provide a minimal working example of an external client that users can: - Test with: pi-pianoteq --client example_client:ExampleClient - Use as a template for custom client development - Reference when following the custom client documentation The example demonstrates all required Client abstract methods with simple console output, making it easy to understand the client lifecycle.
tlsim
force-pushed
the
claude/add-plan-md-file-018ndfS16yeoFxPVKX2H4pNV
branch
from
8271188 to
aa1b2b7
Compare
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
Replace hardcoded --cli flag with flexible client discovery system.
Breaking Changes:
Features:
Implementation:
Documentation: