Ubuntu Clock App

Merge lp://staging/~mzanetti/ubuntu-clock-app/detect-qtmm-version into lp://staging/ubuntu-clock-app

detect-qtmm-version
Merge into trunk

Proposed by Michael Zanetti on 2015-10-21

Status:

Rejected

Rejected by:

Bartosz Kosiorek on 2015-12-10

Proposed branch:

lp://staging/~mzanetti/ubuntu-clock-app/detect-qtmm-version

Merge into:

lp://staging/ubuntu-clock-app

Diff against target:

74 lines (+38/-2)

3 files modified

app/alarm/AlarmSound.qml (+2/-2)
app/components/AlarmAudio.qml (+35/-0)
app/components/CMakeLists.txt (+1/-0)

To merge this branch:

bzr merge lp://staging/~mzanetti/ubuntu-clock-app/detect-qtmm-version

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)			Abstain on 2015-11-04
Ubuntu Phone Apps Jenkins Bot	continuous-integration		Needs Fixing on 2015-10-28
Jenkins Bot	continuous-integration		Approve on 2015-10-27
Michał Sawicz (community)			Abstain on 2015-10-22
Bartosz Kosiorek		2015-10-21	Needs Information on 2015-10-22
Review via email: mp+275177@code.staging.launchpad.net

Commit message

add a hack to detect the available QtMultimedia version

This is supposed to handle API changes in our QtMultimedia distro-patches
without requiring to go through a deprecation phase

Description of the change

This would be the theoretical correct approach forward if we want the alarm sound preview to reflect the alarm volume. However, as the alarm volume seems to be restored to what the setting in the clock app is before the alarm is triggered, the current alarm role volume does not necessarily reflect the upcoming alarm's volume. Because of this, it is arguable how much sense it makes to include this workaround.

A simpler workaround to the api breakage problem, with roughly the same outcome can be found here:
https://code.launchpad.net/~mzanetti/ubuntu-clock-app/drop-audioRole/+merge/275179

lp://staging/~mzanetti/ubuntu-clock-app/detect-qtmm-version updated on 2015-10-21

402. By Michael Zanetti on 2015-10-21: make api a little better

Revision history for this message

Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote on 2015-10-21:

PASSED: Continuous integration, rev:402
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/10/
Executed test runs:
None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/40/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/10/rebuild

review: Approve (continuous-integration)

Revision history for this message

Bartosz Kosiorek (gang65) wrote on 2015-10-22:

I tested it and it is working correctly for me on BQ E4.5 device.
It is nice improvement especially for QT 5.5 upgrade

One inline question below.

Do you think it could help in missing volume changes, during alarm preview?:
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-volume-preview/+merge/264783

Ricardo Salveti already investigated that issue, unfortunately without success.

review: Needs Information

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2015-10-22:

fixed the inline comment.

For me, the volume controls during preview were working fine. Probably the issue was because before it was using the "alert" role, which seems to be wrong in any case.

As written in the description, I'm not really sure if we should merge this branch, or instead just go with the much simpler https://code.launchpad.net/~mzanetti/ubuntu-clock-app/drop-audioRole/+merge/275179

The outcome is pretty much the same with the current audio system.

lp://staging/~mzanetti/ubuntu-clock-app/detect-qtmm-version updated on 2015-10-22

403. By Michael Zanetti on 2015-10-22: s/print/console.log/

Revision history for this message

Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote on 2015-10-22:

FAILED: Continuous integration, rev:403
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/11/
Executed test runs:
None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/41/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/11/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Bartosz Kosiorek (gang65) wrote on 2015-10-22:

@Michael
I would like to clarify, that volume preview is working perfectly on Desktop,
unfortunately it is not working on Phone.

https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-volume-preview/+merge/264783

This is very strange issue. Unfortunately these patches not resolve volume preview issue.
Do you have other idea, why it is not working on Phone?

review: Needs Information

Revision history for this message

Michał Sawicz (saviq) wrote on 2015-10-22:

@Bartosz, it's likely related to bug #1485522, and also the fact that indicator-datetime, AFAIK, sets the volume to the one set in the clock app when playing an alarm sound.

In any case, I vote for the competing approach for now - until we clear the whole volume situation up.

review: Disapprove

Revision history for this message

Albert Astals Cid (aacid) on 2015-10-22:

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2015-10-22:

I'm abstainig from this after all, see my reasoning on the counter-proposal:

https://code.launchpad.net/~mzanetti/ubuntu-clock-app/drop-audioRole/+merge/275179/comments/695921