Skip to content
This repository has been archived by the owner on Jul 7, 2022. It is now read-only.

Use speech recognition from the game screen #301

Merged
merged 56 commits into from May 5, 2022
Merged

Use speech recognition from the game screen #301

merged 56 commits into from May 5, 2022

Conversation

alexandrepiveteau
Copy link
Contributor

@alexandrepiveteau alexandrepiveteau commented May 4, 2022 •
edited

This PR uses the work of #201 and introduces some abstractions to simplify testing :

  • SpeechFacade is a facade which provides access to the speech recognition facilities.
  • SpeechRecognizerFactory is a factory of SpeechRecognizer, which is used by SpeechFacade.

BadrTad and others added 30 commits April 1, 2022 11:45
@BadrTad
Start a SpeechRecognizer screen.
2ee06e8 
  -Integrate SpeechRecognizer(callback) in JetPackCompose (using flows)
@BadrTad
Merge branch 'main' into feature/bt/speech_recognition
49b2133
@BadrTad
Format
c79fb6b
@BadrTad
Merge remote-tracking branch 'origin/feature/bt/speech_recognition' i…
b996de7 
…nto feature/bt/speech_recognition
@BadrTad
First working version of speech recognizer
3b303b0
@BadrTad
Increase Number of results to 10
5701942
@BadrTad
Merge branch 'main' into feature/bt/speech_recognition
4deaafa
@BadrTad
Fix toml by removing duplicate accompanist value
613caae
@BadrTad
Resolve comments:
881d8bc 
  - Delete unused class
  - Rename dependency in toml
  - Add comment about route
  - Refactor to MaxResultsCount
  - use rememberCoroutineScope
@BadrTad
Merge branch 'main' into feature/bt/speech_recognition
7385052
@BadrTad
Use already defined mic icons
567dde9
@BadrTad
Merge remote-tracking branch 'origin/feature/bt/speech_recognition' i…
fe6d605 
…nto feature/bt/speech_recognition
@BadrTad
Use already defined mic icons
ca558c2
@BadrTad @alexandrepiveteau
Update mobile/src/main/java/ch/epfl/sdp/mobile/state/StatefulHome.kt
7392e24 
Co-authored-by: Alexandre Piveteau <alexandre@piveteau.email>
@BadrTad
Make constants private
4ab1fe3
@BadrTad
Add tests and revise permissions in manifest
184455d 
   - Add stateful screen
   - Add tests
   - Make strings consts
@BadrTad
Pass modifier from state screen to screen
c40fb01
@BadrTad
Reformat
9ff589e
@BadrTad
Add tests, doc and refactor
36fd039 
  - Add mutator
  - Extract permission in state
  - Extract strings
@BadrTad
Format
40031fc
@BadrTad
Merge branch 'main' into feature/bt/speech_recognition
6599ac7
@BadrTad
Fix test by changing permission to Record audio
1fc3521
@BadrTad
Merge remote-tracking branch 'origin/feature/bt/speech_recognition' i…
1a18299 
…nto feature/bt/speech_recognition
@BadrTad
Refresh CI
f302b30
@BadrTad
Merge branch 'main' into feature/bt/speech_recognition
a8a3764
@BadrTad
Add more tests to increase coverage
6edbe7e
@BadrTad
Merge remote-tracking branch 'origin/feature/bt/speech_recognition' i…
c467ade 
…nto feature/bt/speech_recognition
@BadrTad
Refresh Cysrus
2246da6
@BadrTad
Refactor code and make classes testable
87ca37b
@BadrTad
Add test comments
9f11201
@KurohanaJuri
Copy link
Contributor

KurohanaJuri commented May 5, 2022 •
edited

I tried it, I have an error 7 : ERROR_NO_MATCH :( (Maybe I tested in a noisy environment and the recognizer didn't work well)

And in the log I have : /ch.epfl.sdp.mobile E/SpeechRecognizer: not connected to the recognition service(Run a second time, and the message didn't show up again)

Can anyone test it on their device ?

Solved

BadrTad
BadrTad reviewed May 5, 2022
View reviewed changes
Copy link
Contributor

@BadrTad BadrTad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job ! The refactoring has indeed made testing more flexible. I did not knew of this pattern before so it was very informative 👍
Just some few suggestions and remarks the rest is good for me

mobile/src/main/java/ch/epfl/sdp/mobile/application/speech/SpeechFacade.kt Show resolved Hide resolved
...st/java/ch/epfl/sdp/mobile/test/infrastructure/speech/android/AndroidSpeechRecognizerTest.kt Show resolved Hide resolved
mobile/src/main/java/ch/epfl/sdp/mobile/state/StatefulGameScreen.kt Show resolved Hide resolved
@alexandrepiveteau
Copy link
Contributor Author

I have a question about something that is not about the code, but the features itself. How can I integrate the voice recognition and the AR ?

If I remember correctly, we want to be able to play with the voice recognition in the AR view. And with this implementation, we need to click on a button to stack the recognition. A solution is to add a button on the AR screen, but is not really hand free as we have defined.

(We can discuss this during our meeting to have other teammate opinion)

After some discussions during our meeting, we've made the following decisions :

  • For the moment, we'll keep the SpeechFacade as-is, and focus on processing single text inputs and applying the moves to the game.
  • At some point in the future, we'll refactor the SpeechFacade so the user may toggle speech recognition on / off and get continuous results. The AR screen will also be augmented with a dedicated speech recognition button.

BadrTad
BadrTad previously approved these changes May 5, 2022
View reviewed changes
@BadrTad
Copy link
Contributor

BadrTad commented May 5, 2022

Maybe for another enhancement PR. But I find it confusing when the snack bar is showing results one by one, and the text next to the mic still says listening !
Not a big deal since this is just for debugging but it something to be aware of 🤔

KurohanaJuri
KurohanaJuri previously approved these changes May 5, 2022
View reviewed changes
KurohanaJuri
KurohanaJuri previously approved these changes May 5, 2022
View reviewed changes
@alexandrepiveteau @BadrTad
Update mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/state/Stat…
74da8d8 
…efulGameScreenTest.kt

Co-authored-by: BadrTad <56789776+BadrTad@users.noreply.github.com>
@alexandrepiveteau @BadrTad
Update mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/state/Stat…
a837111 
…efulGameScreenTest.kt

Co-authored-by: BadrTad <56789776+BadrTad@users.noreply.github.com>
@alexandrepiveteau alexandrepiveteau requested review from KurohanaJuri and BadrTad and removed request for BadrTad May 5, 2022 14:37
@codeclimate
Copy link

codeclimate bot commented May 5, 2022

Code Climate has analyzed commit a837111 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 99.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 97.7% (0.0% change).

View more on Code Climate.

@alexandrepiveteau alexandrepiveteau merged commit 2831899 into main May 5, 2022
@alexandrepiveteau alexandrepiveteau deleted the feature/ap-bt/speech_recognition branch May 5, 2022 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KurohanaJuri KurohanaJuri KurohanaJuri approved these changes

@BadrTad BadrTad BadrTad approved these changes

@matt989253 matt989253 Awaiting requested review from matt989253

@barmettlerl barmettlerl Awaiting requested review from barmettlerl

@Fouad-sys Fouad-sys Awaiting requested review from Fouad-sys

Assignees

@alexandrepiveteau alexandrepiveteau

Labels
update-me-please Right to update current branch with main
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Simple prototype of voice recognition
3 participants
@alexandrepiveteau @KurohanaJuri @BadrTad