Merge lp:~fboucault/camera-app/timer_disable_camera into lp:camera-app/staging

Proposed by Florian Boucault
Status: Merged
Approved by: Ugo Riboni
Approved revision: 661
Merged at revision: 663
Proposed branch: lp:~fboucault/camera-app/timer_disable_camera
Merge into: lp:camera-app/staging
Diff against target: 197 lines (+75/-7)
5 files modified
ViewFinderOverlay.qml (+11/-6)
ViewFinderView.qml (+1/-0)
camera-app.qml (+3/-1)
tests/autopilot/camera_app/emulators/main_window.py (+4/-0)
tests/autopilot/camera_app/tests/test_capture.py (+56/-0)
To merge this branch: bzr merge lp:~fboucault/camera-app/timer_disable_camera
Reviewer Review Type Date Requested Status
Ugo Riboni (community) Needs Fixing
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+288304@code.launchpad.net

Commit message

Disable controls and prevent navigation while a delayed/timed shoot is ongoing.

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
Ugo Riboni (uriboni) wrote :

I have to swipe twice while the timer is running to reach the photo roll. The first swipe stops the counter, the second moves to the photo roll. This sounds confusing to me and I am not sure if it is what you intended.

You added an AP test for timed shooting, which is good, but you did not add a test for the feature in this MR.

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote :

> I have to swipe twice while the timer is running to reach the photo roll. The
> first swipe stops the counter, the second moves to the photo roll. This sounds
> confusing to me and I am not sure if it is what you intended.
>

It is. We don't have (yet) a cancellation button for the timer.

> You added an AP test for timed shooting, which is good, but you did not add a
> test for the feature in this MR.

I did add tests for most of the features in this MR (disabling the various) controls. Not being able to swipe is not tested indeed.

Revision history for this message
Florian Boucault (fboucault) wrote :

> > I have to swipe twice while the timer is running to reach the photo roll.
> The
> > first swipe stops the counter, the second moves to the photo roll. This
> sounds
> > confusing to me and I am not sure if it is what you intended.
> >
>
> It is. We don't have (yet) a cancellation button for the timer.
>
>
> > You added an AP test for timed shooting, which is good, but you did not add
> a
> > test for the feature in this MR.
>
> I did add tests for most of the features in this MR (disabling the various)
> controls. Not being able to swipe is not tested indeed.

Actually, scrap that, the test also checks that swipe is disabled.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ViewFinderOverlay.qml'
2--- ViewFinderOverlay.qml 2016-02-23 11:46:52 +0000
3+++ ViewFinderOverlay.qml 2016-03-07 15:00:51 +0000
4@@ -289,6 +289,7 @@
5 id: bottomEdgeClose
6 anchors.fill: parent
7 onClicked: optionsOverlayClose()
8+ enabled: !camera.timedCaptureInProgress
9 }
10
11 OrientationHelper {
12@@ -305,7 +306,7 @@
13 height: optionsOverlayLoader.height
14 onOpenedChanged: optionsOverlayLoader.item.closeValueSelector()
15 enabled: camera.videoRecorder.recorderState == CameraRecorder.StoppedState
16- && !camera.photoCaptureInProgress
17+ && !camera.photoCaptureInProgress && !camera.timedCaptureInProgress
18 opacity: enabled ? 1.0 : 0.3
19
20 /* At startup, opened is false and 'bottomEdge.height' is 0 until
21@@ -652,13 +653,15 @@
22 enabled: visible
23
24 function timedShoot(secs) {
25+ camera.timedCaptureInProgress = true;
26 timedShootFeedback.start();
27 shootingTimer.remainingSecs = secs;
28 shootingTimer.start();
29 }
30
31 function cancelTimedShoot() {
32- if (shootingTimer.running) {
33+ if (camera.timedCaptureInProgress) {
34+ camera.timedCaptureInProgress = false;
35 shootingTimer.stop();
36 timedShootFeedback.stop();
37 }
38@@ -774,6 +777,7 @@
39 onTriggered: {
40 if (remainingSecs == 0) {
41 running = false;
42+ camera.timedCaptureInProgress = false;
43 controls.shoot();
44 timedShootFeedback.stop();
45 } else {
46@@ -819,7 +823,7 @@
47 iconName: (camera.captureMode == Camera.CaptureStillImage) ? "camcorder" : "camera-symbolic"
48 onClicked: controls.changeRecordMode()
49 enabled: camera.videoRecorder.recorderState == CameraRecorder.StoppedState && !main.contentExportMode
50- && !camera.photoCaptureInProgress
51+ && !camera.photoCaptureInProgress && !camera.timedCaptureInProgress
52 }
53
54 ShootButton {
55@@ -833,6 +837,7 @@
56 }
57
58 enabled: viewFinderOverlay.readyForCapture && !storageMonitor.diskSpaceCriticallyLow
59+ && !camera.timedCaptureInProgress
60 state: (camera.captureMode == Camera.CaptureVideo) ?
61 ((camera.videoRecorder.recorderState == CameraRecorder.StoppedState) ? "record_off" : "record_on") :
62 "camera"
63@@ -869,7 +874,7 @@
64 }
65
66 enabled: !camera.switchInProgress && camera.videoRecorder.recorderState == CameraRecorder.StoppedState
67- && !camera.photoCaptureInProgress
68+ && !camera.photoCaptureInProgress && !camera.timedCaptureInProgress
69 iconName: "camera-flip"
70 onClicked: controls.switchCamera()
71 }
72@@ -892,7 +897,7 @@
73 property real maximumScale: 3.0
74 property bool active: false
75
76- enabled: !camera.photoCaptureInProgress
77+ enabled: !camera.photoCaptureInProgress && !camera.timedCaptureInProgress
78 onPinchStarted: {
79 active = true;
80 initialZoom = zoomControl.value;
81@@ -918,7 +923,7 @@
82 rightMargin: -bottomEdgeIndicators.height
83 }
84 enabled: camera.focus.isFocusPointModeSupported(Camera.FocusPointCustom) &&
85- !camera.photoCaptureInProgress
86+ !camera.photoCaptureInProgress && !camera.timedCaptureInProgress
87 onClicked: {
88 camera.manualFocus(mouse.x, mouse.y);
89 mouse.accepted = false;
90
91=== modified file 'ViewFinderView.qml'
92--- ViewFinderView.qml 2016-02-23 11:46:52 +0000
93+++ ViewFinderView.qml 2016-03-07 15:00:51 +0000
94@@ -96,6 +96,7 @@
95 property alias maximumZoom: camera.maximumDigitalZoom
96 property bool switchInProgress: false
97 property bool photoCaptureInProgress: false
98+ property bool timedCaptureInProgress: false
99
100 onPhotoCaptureInProgressChanged: {
101 if (main.contentExportMode && camera.photoCaptureInProgress) {
102
103=== modified file 'camera-app.qml'
104--- camera-app.qml 2016-02-23 11:46:52 +0000
105+++ camera-app.qml 2016-03-07 15:00:51 +0000
106@@ -135,7 +135,9 @@
107 }
108 }
109 ]
110- interactive: !viewFinderView.touchAcquired && !galleryView.touchAcquired && !viewFinderView.camera.photoCaptureInProgress
111+ interactive: !viewFinderView.touchAcquired && !galleryView.touchAcquired
112+ && !viewFinderView.camera.photoCaptureInProgress
113+ && !viewFinderView.camera.timedCaptureInProgress
114
115 Component.onCompleted: {
116 // FIXME: workaround for qtubuntu not returning values depending on the grid unit definition
117
118=== modified file 'tests/autopilot/camera_app/emulators/main_window.py'
119--- tests/autopilot/camera_app/emulators/main_window.py 2015-11-25 17:00:31 +0000
120+++ tests/autopilot/camera_app/emulators/main_window.py 2016-03-07 15:00:51 +0000
121@@ -90,6 +90,10 @@
122 """Returns the video resolution button of the camera"""
123 return self.get_option_button("videoResolution")
124
125+ def get_timer_delay_button(self):
126+ """Returns the timer delay option button of the camera"""
127+ return self.get_option_button("selfTimerDelay")
128+
129 def get_stop_watch(self):
130 """Returns the stop watch when using the record button of the camera"""
131 return self.app.wait_select_single("StopWatch")
132
133=== modified file 'tests/autopilot/camera_app/tests/test_capture.py'
134--- tests/autopilot/camera_app/tests/test_capture.py 2015-12-01 09:03:34 +0000
135+++ tests/autopilot/camera_app/tests/test_capture.py 2016-03-07 15:00:51 +0000
136@@ -83,6 +83,62 @@
137 # check that the camera is able to capture another photo
138 self.assertThat(exposure_button.enabled, Eventually(Equals(True)))
139
140+ """Test taking a picture with a timer set"""
141+ def test_take_picture_with_timer(self):
142+ delay = 5
143+ self.enable_timer("%s seconds" % str(delay))
144+
145+ # start timed shoot
146+ shoot_button = self.main_window.get_exposure_button()
147+ self.assertThat(shoot_button.enabled, Eventually(Equals(True)))
148+ self.pointing_device.move_to_object(shoot_button)
149+ self.pointing_device.click()
150+
151+ switch_cameras_button = self.main_window.get_swap_camera_button()
152+ record_mode_button = self.main_window.get_record_control()
153+ view_switcher = self.main_window.get_view_switcher()
154+
155+ # controls and navigation should be disabled at this point
156+ self.assertThat(shoot_button.enabled,
157+ Eventually(Equals(True)))
158+ self.assertThat(switch_cameras_button.enabled,
159+ Eventually(Equals(True)))
160+ self.assertThat(record_mode_button.enabled,
161+ Eventually(Equals(True)))
162+ self.assertThat(view_switcher.interactive,
163+ Eventually(Equals(True)))
164+
165+ # after the delay controls and navigation should be re-enabled
166+ self.assertThat(shoot_button.enabled,
167+ Eventually(Equals(True), timeout=delay))
168+ self.assertThat(switch_cameras_button.enabled,
169+ Eventually(Equals(True), timeout=delay))
170+ self.assertThat(record_mode_button.enabled,
171+ Eventually(Equals(True), timeout=delay))
172+ self.assertThat(view_switcher.interactive,
173+ Eventually(Equals(True), timeout=delay))
174+
175+ def enable_timer(self, label_value):
176+ # open bottom edge
177+ bottom_edge = self.main_window.get_bottom_edge()
178+ bottom_edge.open()
179+
180+ # open video resolution option value selector showing the possible
181+ # values
182+ timer_delay_button = self.main_window.get_timer_delay_button()
183+ self.pointing_device.move_to_object(timer_delay_button)
184+ self.pointing_device.click()
185+ option_value_selector = self.main_window.get_option_value_selector()
186+ self.assertThat(
187+ option_value_selector.visible, Eventually(Equals(True)))
188+
189+ # select a 5 seconds delay
190+ option = self.main_window.get_option_value_button(label_value)
191+ self.pointing_device.move_to_object(option)
192+ self.pointing_device.click()
193+
194+ bottom_edge.close()
195+
196 def test_record_video(self):
197 """Test clicking on the record control.
198

Subscribers

People subscribed via source and target branches