-
Notifications
You must be signed in to change notification settings - Fork 325
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
tools: demo-gui: improvements #9819
Conversation
iuliana-prodan
commented
Feb 7, 2025
- Improve error handling and readability
- Refactor GUI initialization and structure
- Add stop and mute functionality to playback
- Update GUI color scheme
Here's a (not so good) video of the sof-demo-gui. |
normalized_volume = user_volume / 100.0 | ||
scaled_volume = 31 * math.sqrt(normalized_volume) | ||
return int(round(scaled_volume)) | ||
except Exception as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would the above raise an exception? Only if normalized_volume < 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but it's kind of impossible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, so maybe it isn't needed? Also, I'm not an expert in python, but @marc-hb pointed out in a few cases, that over broad exception catching might actually not be a very good idea. It has a potential of hiding errors instead of causing the program to fail to prompt a proper investigation of the problem, in particular when all exceptions are caught, not some specific ones, which we know that we want to react to in a non-default way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has a potential of hiding errors instead of causing the program to fail to prompt a proper investigation of the problem, in particular when all exceptions are caught, not some specific ones, which we know that we want to react to in a non-default way
W0178
https://pylint.pycqa.org/en/latest/user_guide/messages/warning/broad-exception-caught.html
pylint
is not always right but it is right most of the time and it should always be used. It also offers plenty of ways to silence specific warnings, try git grep pylint
for examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has a potential of hiding errors instead of causing the program to fail to prompt a proper investigation of the problem, in particular when all exceptions are caught, not some specific ones, which we know that we want to react to in a non-default way
W0178
https://pylint.pycqa.org/en/latest/user_guide/messages/warning/broad-exception-caught.html
pylint
is not always right but it is right most of the time and it should always be used. It also offers plenty of ways to silence specific warnings, trygit grep pylint
for examples.
Thanks @marc-hb.
I'll have a look!
…ller engine Improve error handling and readability by: - adding try-except blocks for better error handling; - enhancing error messages and checks; - removing redundant print statements. Signed-off-by: Iuliana Prodan <[email protected]>
Added apply_css() and init_ui() methods for better organization. Implemented create_control_frame(), create_file_frame(), and create_record_frame() methods to improve readability and maintainability. Signed-off-by: Iuliana Prodan <[email protected]>
Added handle_stop() function to stop playback. Updated execute_command() to support the stop command. Added Stop button to GUI and linked it to on_stop_clicked() method. Signed-off-by: Iuliana Prodan <[email protected]>
Introduced handle_mute() function to toggle audio mute state. Updated execute_command() to include mute command. Added mute button to GUI and linked it to on_mute_clicked() method. Signed-off-by: Iuliana Prodan <[email protected]>
Update GUI color scheme for improved readability. Signed-off-by: Iuliana Prodan <[email protected]>
dc6ac3c
to
3968efe
Compare
Changes in v2:
|