Merge lp:~xavi-garcia-mena/indicator-sound/bug-1555831-get-root-icon into lp:indicator-sound/15.10

Proposed by Xavi Garcia
Status: Merged
Approved by: Charles Kerr
Approved revision: 534
Merged at revision: 534
Proposed branch: lp:~xavi-garcia-mena/indicator-sound/bug-1555831-get-root-icon
Merge into: lp:indicator-sound/15.10
Diff against target: 201 lines (+116/-41)
4 files modified
src/service.vala (+9/-41)
tests/integration/indicator-sound-test-base.cpp (+43/-0)
tests/integration/indicator-sound-test-base.h (+2/-0)
tests/integration/test-indicator.cpp (+62/-0)
To merge this branch: bzr merge lp:~xavi-garcia-mena/indicator-sound/bug-1555831-get-root-icon
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+289203@code.launchpad.net

Commit message

Changed to code to get the root icon. Now it does not take into account the source output, as in fact that code was not differentiating between any particular case.
The code was added in the past in the case that we should differentiate between bluetooth, headphones, etc... but it was never used.

Description of the change

Changed to code to get the root icon. Now it does not take into account the source output, as in fact that code was not differentiating between any particular case.
The code was added in the past in the case that we should differentiate between bluetooth, headphones, etc... but it was never used.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Looks very good to me.

Not an issue with this MP, but rather just thinking out loud -- is there anything analogous to the Menu Matcher harness for GActions? Extracting these states out is repetitive and wastes a lot of LOC.

review: Approve
Revision history for this message
Xavi Garcia (xavi-garcia-mena) wrote :

Thanks for the review, Charles.

I don't think we have anything like gmenuharness yet (will ask Pete), but I agree: it would be good to have something similar for actions...

535. By Xavi Garcia

Added volume check to root icon test

Revision history for this message
Charles Kerr (charlesk) wrote :

reapproved with r535

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/service.vala'
--- src/service.vala 2016-03-02 13:50:43 +0000
+++ src/service.vala 2016-03-17 10:07:26 +0000
@@ -290,48 +290,16 @@
290290
291 private bool block_info_notifications = false;291 private bool block_info_notifications = false;
292292
293 private static unowned string get_volume_root_icon_by_volume (double volume, VolumeControl.ActiveOutput active_output) {
294 switch (active_output) {
295 case VolumeControl.ActiveOutput.SPEAKERS:
296 case VolumeControl.ActiveOutput.HEADPHONES:
297 case VolumeControl.ActiveOutput.BLUETOOTH_HEADPHONES:
298 case VolumeControl.ActiveOutput.BLUETOOTH_SPEAKER:
299 case VolumeControl.ActiveOutput.USB_SPEAKER:
300 case VolumeControl.ActiveOutput.USB_HEADPHONES:
301 case VolumeControl.ActiveOutput.HDMI_SPEAKER:
302 case VolumeControl.ActiveOutput.HDMI_HEADPHONES:
303 if (volume <= 0.0)
304 return "audio-volume-muted-panel";
305 if (volume <= 0.3)
306 return "audio-volume-low-panel";
307 if (volume <= 0.7)
308 return "audio-volume-medium-panel";
309 return "audio-volume-high-panel";
310
311 default:
312 return "";
313 }
314 }
315
316 private unowned string get_volume_root_icon (double volume, bool mute, VolumeControl.ActiveOutput active_output) {293 private unowned string get_volume_root_icon (double volume, bool mute, VolumeControl.ActiveOutput active_output) {
317 switch (active_output) {294 if (mute || volume <= 0.0)
318 case VolumeControl.ActiveOutput.SPEAKERS:295 return this.mute_blocks_sound ? "audio-volume-muted-blocking-panel" : "audio-volume-muted-panel";
319 case VolumeControl.ActiveOutput.HEADPHONES:296 if (this.accounts_service != null && this.accounts_service.silentMode)
320 case VolumeControl.ActiveOutput.BLUETOOTH_HEADPHONES:297 return "audio-volume-muted-panel";
321 case VolumeControl.ActiveOutput.BLUETOOTH_SPEAKER:298 if (volume <= 0.3)
322 case VolumeControl.ActiveOutput.USB_SPEAKER:299 return "audio-volume-low-panel";
323 case VolumeControl.ActiveOutput.USB_HEADPHONES:300 if (volume <= 0.7)
324 case VolumeControl.ActiveOutput.HDMI_SPEAKER:301 return "audio-volume-medium-panel";
325 case VolumeControl.ActiveOutput.HDMI_HEADPHONES:302 return "audio-volume-high-panel";
326 if (mute || volume <= 0.0)
327 return this.mute_blocks_sound ? "audio-volume-muted-blocking-panel" : "audio-volume-muted-panel";
328 if (this.accounts_service != null && this.accounts_service.silentMode)
329 return "audio-volume-muted-panel";
330 return get_volume_root_icon_by_volume (volume, active_output);
331
332 default:
333 return "";
334 }
335 }303 }
336304
337 private void update_notification () {305 private void update_notification () {
338306
=== modified file 'tests/integration/indicator-sound-test-base.cpp'
--- tests/integration/indicator-sound-test-base.cpp 2016-03-04 15:25:39 +0000
+++ tests/integration/indicator-sound-test-base.cpp 2016-03-17 10:07:26 +0000
@@ -989,6 +989,49 @@
989 return QVariantList();989 return QVariantList();
990}990}
991991
992QStringList IndicatorSoundTestBase::getRootIconValue(bool *isValid)
993{
994 QString result = 0;
995
996 QVariantList varList = getActionValue("root");
997 if (isValid != nullptr)
998 {
999 *isValid = false;
1000 }
1001 if (varList.at(0).canConvert<QDBusArgument>())
1002 {
1003 const QDBusArgument dbusArg = qvariant_cast<QDBusArgument>(varList.at(0));
1004 if (dbusArg.currentType() == QDBusArgument::MapType)
1005 {
1006 QVariantMap map;
1007 dbusArg >> map;
1008 QVariantMap::const_iterator iter = map.find("icon");
1009 if (iter != map.end())
1010 {
1011 const QDBusArgument iconArg = qvariant_cast<QDBusArgument>((*iter));
1012 if (iconArg.currentType() == QDBusArgument::StructureType)
1013 {
1014 QString name;
1015 QVariant iconValue;
1016 iconArg.beginStructure();
1017 iconArg >> name;
1018 iconArg >> iconValue;
1019 if (name == "themed" && iconValue.type() == QVariant::StringList)
1020 {
1021 if (isValid != nullptr)
1022 {
1023 *isValid = true;
1024 }
1025 }
1026 iconArg.endStructure();
1027 return iconValue.toStringList();
1028 }
1029 }
1030 }
1031 }
1032 return QStringList();
1033}
1034
992qlonglong IndicatorSoundTestBase::getVolumeSyncValue(bool *isValid)1035qlonglong IndicatorSoundTestBase::getVolumeSyncValue(bool *isValid)
993{1036{
994 qlonglong result = 0;1037 qlonglong result = 0;
9951038
=== modified file 'tests/integration/indicator-sound-test-base.h'
--- tests/integration/indicator-sound-test-base.h 2016-03-04 15:25:39 +0000
+++ tests/integration/indicator-sound-test-base.h 2016-03-17 10:07:26 +0000
@@ -148,6 +148,8 @@
148148
149 qlonglong getVolumeSyncValue(bool *isValid = nullptr);149 qlonglong getVolumeSyncValue(bool *isValid = nullptr);
150150
151 QStringList getRootIconValue(bool *isValid = nullptr);
152
151 float getVolumeValue(bool *isValid = nullptr);153 float getVolumeValue(bool *isValid = nullptr);
152154
153 static QVariant waitPropertyChanged(QSignalSpy * signalSpy, QString property);155 static QVariant waitPropertyChanged(QSignalSpy * signalSpy, QString property);
154156
=== modified file 'tests/integration/test-indicator.cpp'
--- tests/integration/test-indicator.cpp 2016-03-04 15:25:39 +0000
+++ tests/integration/test-indicator.cpp 2016-03-17 10:07:26 +0000
@@ -32,6 +32,68 @@
32{32{
33};33};
3434
35TEST_F(TestIndicator, PhoneCheckRootIcon)
36{
37 double INITIAL_VOLUME = 0.0;
38
39 ASSERT_NO_THROW(startAccountsService());
40 EXPECT_TRUE(clearGSettingsPlayers());
41 ASSERT_NO_THROW(startPulsePhone());
42
43 // initialize volumes in pulseaudio
44 EXPECT_TRUE(setStreamRestoreVolume("alert", INITIAL_VOLUME));
45
46 // start now the indicator, so it picks the new volumes
47 ASSERT_NO_THROW(startIndicator());
48
49 // check that the volume is set and give
50 // time to the indicator to start
51 EXPECT_MATCHRESULT(mh::MenuMatcher(phoneParameters())
52 .item(mh::MenuItemMatcher()
53 .action("indicator.root")
54 .string_attribute("x-canonical-type", "com.canonical.indicator.root")
55 .string_attribute("x-canonical-scroll-action", "indicator.scroll")
56 .string_attribute("x-canonical-secondary-action", "indicator.mute")
57 .string_attribute("submenu-action", "indicator.indicator-shown")
58 .mode(mh::MenuItemMatcher::Mode::all)
59 .submenu()
60 .item(mh::MenuItemMatcher()
61 .section()
62 .item(silentModeSwitch(false))
63 .item(volumeSlider(INITIAL_VOLUME, "Volume"))
64 )
65 .item(mh::MenuItemMatcher()
66 .label("Sound Settingsā€¦")
67 .action("indicator.phone-settings")
68 )
69 ).match());
70
71 QStringList mutedIcon = {"audio-volume-muted-panel", "audio-volume-muted", "audio-volume", "audio"};
72 EXPECT_EQ(getRootIconValue(), mutedIcon);
73
74 QStringList lowVolumeIcon = {"audio-volume-low-panel", "audio-volume-low", "audio-volume", "audio"};
75 for( double volume = 0.1; volume <= 0.3; volume+=0.1)
76 {
77 EXPECT_TRUE(setStreamRestoreVolume("alert", volume));
78 EXPECT_EQ(getRootIconValue(), lowVolumeIcon);
79 }
80 EXPECT_TRUE(setStreamRestoreVolume("alert", 0.4));
81
82 QStringList mediumVolumeIcon = {"audio-volume-medium-panel", "audio-volume-medium", "audio-volume", "audio"};
83 for( double volume = 0.4; volume <= 0.7; volume+=0.1)
84 {
85 EXPECT_TRUE(setStreamRestoreVolume("alert", volume));
86 EXPECT_EQ(getRootIconValue(), mediumVolumeIcon);
87 }
88
89 QStringList highVolumeIcon = {"audio-volume-high-panel", "audio-volume-high", "audio-volume", "audio"};
90 for( double volume = 0.8; volume <= 1.0; volume+=0.1)
91 {
92 EXPECT_TRUE(setStreamRestoreVolume("alert", volume));
93 EXPECT_EQ(getRootIconValue(), highVolumeIcon);
94 }
95}
96
35TEST_F(TestIndicator, PhoneTestExternalMicInOut)97TEST_F(TestIndicator, PhoneTestExternalMicInOut)
36{98{
37 double INITIAL_VOLUME = 0.0;99 double INITIAL_VOLUME = 0.0;

Subscribers

People subscribed via source and target branches