Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3112: Prevent unnecessary tts stop #3113

Merged
merged 2 commits into from
Feb 21, 2025
Merged

3112: Prevent unnecessary tts stop #3113

merged 2 commits into from
Feb 21, 2025

Conversation

steffenkleinle
Copy link
Member

@steffenkleinle steffenkleinle commented Feb 19, 2025

Short Description

Prevent unnecessary tts stop causing errors if no tts engine installed.

Proposed Changes

  • Prevent unnecessary tts stop causing errors if no tts engine installed.
  • Refactor reportError

Side Effects

  • This error will not be sent to sentry anymore. We have 537k events for it, which seems more than enough. I investigated this in the past, but wasn't able to fix it. However, it is handled and not a problem.
  • I thought of preventing sending a sentry stacktrace for known issues (such as this one), but decided against as it still might be helpful to detect issues such as this one. However, we will continue to get this error for people that try to open the tts player but dont have a tts player installed. Due to the unexpected behavior on language change described below, I reverted the previous changes and ignore the error instead.

Testing

Not sure how to test this. Have a degoogled android phone I guess. Or just throw an error in stop and ensure that it is not called without actually opening the tts function.

Resolved Issues

Fixes: #3112


Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Looks like your changes crashed sth on ios. On main everything works fine.
Tested on ios simulator (ios 18.0)

If i click play it reads only the headline and shows "nichts zu lesen" and stops
Guess the updateSentence is broken somehow

Simulator.Screen.Recording.-.iPhone.16.-.2025-02-20.at.10.54.27.mp4

@bahaaTuffaha
Copy link
Contributor

Looks like your changes crashed sth on ios. On main everything works fine. Tested on ios simulator (ios 18.0)

Same for Andorid it just stops with nothing to read...

Copy link
Contributor

@bahaaTuffaha bahaaTuffaha left a comment

Choose a reason for hiding this comment

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

Works as expected on Android...
I don't have a degoogled phone to test it on 😐️

Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Tested on android and ios. 👍
Works fine

@steffenkleinle steffenkleinle merged commit 9105751 into main Feb 21, 2025
7 checks passed
@steffenkleinle steffenkleinle deleted the 3112-tts-stop branch February 21, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: No TTS engine installed
3 participants