search

androidthings / sample-googleassistant

Google Assistant API sample for Android Things
Apache License 2.0
467 stars 147 forks source link

fixes #43 #49

Closed plattysoft closed 6 years ago

plattysoft commented 6 years ago

The LED flashing on audio received causes too many calls to GPIO and that produces audio clipping.

As a solution, I have created a thread that starts one blink each time setBlinking (true) is called, so if several blinks are requested, the are considered as "do one more blink"

To make it a bit more dynamic, the duration of the blink is pseudo-random.

Note that the LED Is opened as ACTIVE_LOW, and therefore false means turn on and true means turns off. You may want to change that.

proppy commented 6 years ago

What was "nice" with the previous behavior was that the blinking was happening in sync with the audio playback.

Maybe we could simply blink every XX samples?

Also we didn't see clipping issue with previous releases, I wonder if this is a regression with the PIO latency with recent Android Things developer preview.

plattysoft commented 6 years ago

@proppy maybe something has changed on the latest release, but on 5.1 it is definitely an issue.

I can't see a way to make it sync with the playback while solving this problem.

plattysoft commented 6 years ago

Other possible solution is to create a layer on the communication with the LED that only changes it if the previous call was longer than X ms ago.

plattysoft commented 6 years ago

I think I addressed all the comments @Fleker, let me know if there is any other request

proppy commented 6 years ago

Other possible solution is to create a layer on the communication with the LED that only changes it if the previous call was longer than X ms ago.

Maybe we could do that in a follow up change? And keep this PR simple by just removing the blinking logic (without replacing it with something else).

It would fix the immediate issue (#43) and gives up more time to design a replacement.

What do you think?