Refactor minimum recording length handling #112
Conversation
…t to debug timeout-related errors
Update unit test to reflect event refactor
|
#76 and #73 added these Timers and tests, but I don't see why we wouldn't wait for the confirmation or playback before proceeding with recording which is what appears to happen now. @JarbasAl @emphasize am I missing something with the Timer implementation? |
| else: | ||
| self.voice_loop.state = ListeningState.BEFORE_COMMAND | ||
| else: | ||
| self.voice_loop.state = ListeningState.BEFORE_COMMAND | ||
| self.voice_loop.reset_speech_timer() |
There was a problem hiding this comment.
why was this moved? what are the implications of moving it?
There was a problem hiding this comment.
This was moved so the timer is reset after the audio playback is completed, so the time it takes to play the confirmation sound isn't factored into the minimum length
| Timer(0.5, lambda: self.voice_loop.confirmation_event.set()).start() | ||
| if not self.voice_loop.confirmation_event.wait(0.5): | ||
| LOG.warning("Confirmation event timed out!") | ||
| self.voice_loop.confirmation_event.set() |
There was a problem hiding this comment.
if this is added here, then doesnt make sense here
i think the idea is to allow the loop to continue uninterrupted instead of blocking for 0.5 seconds, as this event is supposed to come from an external source (ovos-audio)
i would suggest making the timeout configurable
if self.chunk_callback is not None:
self._chunk_info.energy = self.debiased_energy(chunk, self.mic.sample_width)
self.chunk_callback(self._chunk_info)@emphasize should be more familiar with this part of the code and give a better rationale on why it was done this way
There was a problem hiding this comment.
this affects usage of get_response via voice satellites, that was the main reason for introducing this new state in the loop, as get_response would often timeout just because the sat device started listening too late,
if the instant_listening flag is not set then it's supposed to ensure audio is only recorded once the acknowledgment sound is played
|
replaced by #125 |
Move timer reset to ensure minimum recording length is measured from the moment input is accepted
Related to #107
Related to #110
Also reported by a Neon user on Matrix that no input was accepted after WW confirmation sound was played