Refactor the esphome implementation to simplify dependencies and interfaces#72
Merged
Refactor the esphome implementation to simplify dependencies and interfaces#72
Conversation
brownard
force-pushed
the
Refactor_ESPHome
branch
from
648d6ac to
c1eb43b
Compare
…e device to simplify dependencies/testing - Remove voice satellite dependency on server, entities and settings. These only need to be known by the overarching 'device' - Add an injectable device builder for building a voice satellite device from settings - Update tests
- Get/set wake words and muted directly from/to data store to ensure its always the source of truth - Rework voice input flow to recreate the detector when wake word settings change - Remove now unnecessary settings watcher from VoiceSatelliteService
…ce for use with MediaPlayerEntity - Remove properties that are only used internally from the VoiceOutput interface - Add a separate MediaPlayer interface tor use with MediaPlayerEntity - Don't expose the tts/media players, all calls should go through VoiceOutput/MediaPlayer interface - Update StubVoiceOutput and tests
Reflects the naming in other ESPHome implementations and conceptually the voice assistant implementation is just one components of a larger 'voice satellite' device
brownard
force-pushed
the
Refactor_ESPHome
branch
from
c1eb43b to
ff9b8f9
Compare
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
1 participant
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.
The current implementation had the voice assistant as a subclass of a device, which meant it had dependencies on the device settings/server/entitites/etc. This was starting to get unwieldly, particularly when adding more entities, and required lots of stubbing when testing.
Instead prefer composition so that a voice assistant is a child 'component' of a 'voice satellite' device, so that it only needs to handle voice input/output/messages. This simplifies its dependencies and makes it easier to test in isolation.
Additionally the voice input/output interfaces exposed unnecessary properties and didn't properly update settings when changed, requiring the service to watch for changes. Rework them so that they only expose the minimal required interface and automatically update settings when necessary.
Generally simplify names and tidy up.
Planned future refactoring should properly separate the Android specific implementations, e.g. microphone and player implementations, from the esphome implementation to allow it to be pulled out to a, ideally pure JVM, module to allow reuse in e.g. Kotlin Multiplatform code, but this pull request is already big enough for now.