Add volume setting to talk on speaker #2179

bertrandda · 2024-12-03T21:54:46Z

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

If your changes affects code, did your write the tests?
Are tests passing? (npm test on both front/server)
Is the linter passing? (npm run eslint on both front/server)
Did you run prettier? (npm run prettier on both front/server)
If you are adding a new features/services, did you run integration comparator? (npm run compare-translations on front)
Did you test this pull request in real life? With real devices? If this development is a big feature or a new service, we recommend that you provide a Docker image to the community (french forum/english forum) for testing before merging.
If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)
If you are adding a new features/services which needs explanation, did you modify the user documentation? See the GitHub repo and the website.
Did you add fake requests data for the demo mode (front/src/config/demo.js) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Please provide a description of the change here. It's always best with screenshots, so don't hesitate to add some!

I add volume in Talk on Speaker form
We need the right volume level for each situation (alert, information notification, small/large room...)

relativeci · 2024-12-03T22:02:57Z

#2921 Bundle Size — 10.41MiB (+0.01%).

49d69bc(current) vs f961aef master#2919(baseline)

Warning

Bundle contains 3 duplicate packages – View duplicate packages

Bundle metrics

2 changes

1 regression

	Current #2921	Baseline #2919
Initial JS	`5.64MiB`(`+0.02%`)	`5.64MiB`
Initial CSS	`304.89KiB`	`304.89KiB`
Cache Invalidation	`54.06%`	`0%`
Chunks	`51`	`51`
Assets	`173`	`173`
Modules	`1511`	`1511`
Duplicate Modules	`21`	`21`
Duplicate Code	`0.83%`	`0.83%`
Packages	`124`	`124`
Duplicate Packages	`3`	`3`

Bundle size by type

1 change

1 regression

	Current #2921	Baseline #2919
JS	`7.43MiB` (`+0.02%`)	`7.43MiB`
IMG	`2.54MiB`	`2.54MiB`
CSS	`321.79KiB`	`321.79KiB`
Fonts	`93.55KiB`	`93.55KiB`
Other	`17.79KiB`	`17.79KiB`
HTML	`13.58KiB`	`13.58KiB`

Bundle analysis report Branch bertrandda:feat/talk-speaker-vol... Project dashboard

^{Generated by RelativeCI Documentation Report issue}

codecov · 2024-12-03T22:24:27Z

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.53%. Comparing base (6a1779d) to head (49d69bc).
Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
...r/services/google-cast/lib/google_cast.setValue.js	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2179      +/-   ##
==========================================
- Coverage   98.53%   98.53%   -0.01%     
==========================================
  Files         876      876              
  Lines       14409    14428      +19     
==========================================
+ Hits        14198    14216      +18     
- Misses        211      212       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Pierre-Gilles

Hey! Thanks for this PR 🙏
It's great!

One design question: could you put the volume bar full width?

Pierre-Gilles · 2024-12-06T07:45:53Z

server/services/google-cast/lib/google_cast.setValue.js

+          }
+        });
+      }
+


You're not waiting for the callback to finish, are you sure the volume will be changed when client.launch will be started ?

You can use promisify in utils to turn this in a promise for example

Done 👍 . Volume is set immediatly, when we play audio from url it needs time to load it, so I thought is was not necessary

Pierre-Gilles · 2024-12-06T07:46:31Z

server/test/services/sonos/lib/sonos.setValue.test.js

@@ -188,12 +188,12 @@ describe('SonosHandler.setValue', () => {
      read_only: false,
      has_feedback: false,
    };
-    await sonosHandler.setValue(device, deviceFeature, 'http://test.com');
+    await sonosHandler.setValue(device, deviceFeature, 'http://test.com', { volume: 30 });


Can you create a new test to keep the default volume test ?

Pierre-Gilles · 2024-12-06T07:46:40Z

server/test/services/airplay/lib/airplay.setValue.test.js

@@ -85,15 +85,15 @@ describe('AirplayHandler.setValue', () => {
    airplayHandler.scanTimeout = 1;
    const devices = await airplayHandler.scan();
    const device = devices[0];
-    await airplayHandler.setValue(device, device.features[0], 'http://play-url.com');
+    await airplayHandler.setValue(device, device.features[0], 'http://play-url.com', { volume: 30 });


Can you create a new test to keep the default volume test ?

Pierre-Gilles · 2024-12-06T07:53:05Z

server/services/google-cast/lib/google_cast.setValue.js

@@ -22,6 +23,16 @@ async function setValue(device, deviceFeature, value) {
    client.connect(ipAddress, () => {
      logger.debug('Google Cast Connected, launching app ...');

+      if (options.volume) {
+        client.setVolume({ level: options.volume / 100 }, (err, newvol) => {


You're changing the volume for the voice, but what if the user was playing music, will it be super loud after?

I'm wondering if we could get the previous volume with player.getStatus
Then rollback the volume after the announce

What do you think?

We can try, I'll check documentation. How does it work when audio notification is terminated on google device? It come back automatically to previous app?

I have no idea to be honest! :D

Done. But I couldn't do it for Airplay, the lib does not have method to get volume, only set.

bertrandda · 2024-12-08T22:43:52Z

One design question: could you put the volume bar full width?
I try but nothing works, 😅.
It seems to be global, brightness has also the issue

Do you think it can be fixed in @gladysassistant/theme-optimized

Pierre-Gilles · 2024-12-09T07:43:02Z

One design question: could you put the volume bar full width?
I try but nothing works, 😅.

Shit! :D

Do you think it can be fixed in @gladysassistant/theme-optimized

Maybe ^^

Can you try modifying the theme in the node_modules, and if it works, submit a PR here :

https://github.com/GladysAssistant/theme-optimized/blob/master/dashboard.css

?

bertrandda · 2024-12-12T18:53:17Z

Can you try modifying the theme in the node_modules, and if it works, submit a PR here :

https://github.com/GladysAssistant/theme-optimized/blob/master/dashboard.css

PR created GladysAssistant/theme-optimized#3

Add volume setting to play notification

5298664

Fix log

dd7c8dd

Pierre-Gilles requested changes Dec 6, 2024

View reviewed changes

Promisify setVolume

af3be46

bertrandda added 2 commits December 12, 2024 21:14

Rolmback volume for Google cast after notification finished

943b72e

Fix tests

49d69bc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add volume setting to talk on speaker #2179

Add volume setting to talk on speaker #2179

bertrandda commented Dec 3, 2024 •

edited

Loading

relativeci bot commented Dec 3, 2024 •

edited

Loading

codecov bot commented Dec 3, 2024 •

edited

Loading

Pierre-Gilles left a comment

Pierre-Gilles Dec 6, 2024

bertrandda Dec 8, 2024

Pierre-Gilles Dec 6, 2024

Pierre-Gilles Dec 6, 2024

Pierre-Gilles Dec 6, 2024

bertrandda Dec 8, 2024

Pierre-Gilles Dec 9, 2024

bertrandda Dec 12, 2024

bertrandda commented Dec 8, 2024

Pierre-Gilles commented Dec 9, 2024

bertrandda commented Dec 12, 2024

Add volume setting to talk on speaker #2179

Are you sure you want to change the base?

Add volume setting to talk on speaker #2179

Conversation

bertrandda commented Dec 3, 2024 • edited Loading

Pull Request check-list

Description of change

relativeci bot commented Dec 3, 2024 • edited Loading

#2921 Bundle Size — 10.41MiB (+0.01%).

codecov bot commented Dec 3, 2024 • edited Loading

Codecov Report

Pierre-Gilles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bertrandda commented Dec 8, 2024

Pierre-Gilles commented Dec 9, 2024

bertrandda commented Dec 12, 2024

bertrandda commented Dec 3, 2024 •

edited

Loading

relativeci bot commented Dec 3, 2024 •

edited

Loading

codecov bot commented Dec 3, 2024 •

edited

Loading