Merge lp:~zsombi/ubuntu-ui-toolkit/alarm-status into lp:ubuntu-ui-toolkit

Proposed by Zsombor Egri
Status: Merged
Approved by: Zoltan Balogh
Approved revision: 755
Merged at revision: 763
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/alarm-status
Merge into: lp:ubuntu-ui-toolkit
Prerequisite: lp:~zsombi/ubuntu-ui-toolkit/alarm-eds-integration
Diff against target: 440 lines (+136/-35)
10 files modified
components.api (+4/-0)
modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp (+32/-14)
modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h (+1/-0)
modules/Ubuntu/Components/plugin/alarmrequest_p.cpp (+2/-2)
modules/Ubuntu/Components/plugin/alarmrequest_p.h (+7/-1)
modules/Ubuntu/Components/plugin/alarmrequest_p_p.h (+1/-1)
modules/Ubuntu/Components/plugin/ucalarm.cpp (+72/-12)
modules/Ubuntu/Components/plugin/ucalarm.h (+10/-3)
modules/Ubuntu/Components/plugin/ucalarm_p.h (+1/-1)
tests/resources/alarm/Alarms.qml (+6/-1)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/alarm-status
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Cris Dywan Approve
Review via email: mp+186285@code.launchpad.net

Commit message

Fix alarm status reporting, updating documentation on asynchronous behavior of save and cancel operations. Alarm status notification reports the operation the status refers to.

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
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Cris Dywan (kalikiana) wrote :

"Further operations should wait till the previous operation is completed"

Can we fail instead of putting the responsibility on the consumer of the API? The API should bail out if you're making changes while other changes are still happening (probably checking AlarmRequest::InProgress) and return with "Operating in progress". I'm thinking losely of Sqlite which has a similar behavior returning BUSY if you're doing something while another operation is not yet finished.

review: Needs Fixing
Revision history for this message
Zsombor Egri (zsombi) wrote :

> "Further operations should wait till the previous operation is completed"
>
> Can we fail instead of putting the responsibility on the consumer of the API?
> The API should bail out if you're making changes while other changes are still
> happening (probably checking AlarmRequest::InProgress) and return with
> "Operating in progress". I'm thinking losely of Sqlite which has a similar
> behavior returning BUSY if you're doing something while another operation is
> not yet finished.

It actually does bail out. If an operation is in progress, the status is reported to be InProgress, so that happens. The save() cancel() are void, but the status or error reports any success/failure.

Revision history for this message
Cris Dywan (kalikiana) wrote :

I guess the documention is a bit ambiguous (and I tend to disambiguate with a sceptic's hat on) but then this is okay. It makes sense to point out clearly so long as we don't fully rely on developers' goodwill. Thanks for clarifying!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components.api'
2--- components.api 2013-09-24 03:55:53 +0000
3+++ components.api 2013-09-24 06:30:40 +0000
4@@ -624,6 +624,7 @@
5 prototype: "QObject"
6 exports: ["Alarm 0.1"]
7 name: "Status"
8+ name: "Operation"
9 name: "Error"
10 name: "AlarmType"
11 name: "DayOfWeek"
12@@ -636,6 +637,9 @@
13 Property { name: "sound"; type: "QUrl" }
14 Property { name: "error"; type: "int"; isReadonly: true }
15 Property { name: "status"; type: "Status"; isReadonly: true }
16+ Signal {
17+ name: "statusChanged"
18+ Parameter { name: "operation"; type: "Operation" }
19 Method { name: "save" }
20 Method { name: "cancel" }
21 Method { name: "reset" }
22
23=== modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp'
24--- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp 2013-09-12 05:27:40 +0000
25+++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp 2013-09-24 06:30:40 +0000
26@@ -356,7 +356,7 @@
27 QOrganizerItemId itemId = alarm.cookie.value<QOrganizerItemId>();
28 event = AlarmsAdapter::get()->manager->item(itemId);
29 if (event.isEmpty()) {
30- setStatus(AlarmRequest::Fail, UCAlarm::AdaptationError);
31+ setStatus(AlarmRequest::Saving, AlarmRequest::Fail, UCAlarm::AdaptationError);
32 return false;
33 }
34 AlarmsAdapter::get()->updateOrganizerEventFromAlarmData(alarm, event);
35@@ -375,7 +375,7 @@
36 bool AlarmRequestAdapter::remove(AlarmData &alarm)
37 {
38 if (!alarm.cookie.isValid()) {
39- setStatus(AlarmRequest::Fail, UCAlarm::InvalidEvent);
40+ setStatus(AlarmRequest::Canceling, AlarmRequest::Fail, UCAlarm::InvalidEvent);
41 return false;
42 }
43
44@@ -435,7 +435,7 @@
45 }
46 completed = false;
47 // make sure we are in progress state
48- setStatus(AlarmRequest::InProgress);
49+ setStatus(requestTypeToOperation(), AlarmRequest::InProgress);
50 QObject::connect(m_request, SIGNAL(resultsAvailable()), q_ptr, SLOT(_q_updateProgress()));
51 if (m_request->start()) {
52 // check if the request got completed without having the slot called (some engines may do that)
53@@ -455,51 +455,52 @@
54 completed = true;
55
56 QOrganizerAbstractRequest::State state = m_request->state();
57+ AlarmRequest::Operation opCode = requestTypeToOperation();
58 switch (state) {
59 case QOrganizerAbstractRequest::InactiveState: {
60- setStatus(AlarmRequest::Ready);
61+ setStatus(opCode, AlarmRequest::Ready);
62 break;
63 }
64 case QOrganizerAbstractRequest::ActiveState: {
65- setStatus(AlarmRequest::InProgress);
66+ setStatus(opCode, AlarmRequest::InProgress);
67 completed = false;
68 break;
69 }
70 case QOrganizerAbstractRequest::CanceledState: {
71- setStatus(AlarmRequest::Fail, AlarmsAdapter::OrganizerError + m_request->error());
72+ setStatus(opCode, AlarmRequest::Fail, AlarmsAdapter::OrganizerError + m_request->error());
73 break;
74 }
75 case QOrganizerAbstractRequest::FinishedState: {
76 int code = m_request->error();
77 if (code != QOrganizerManager::NoError) {
78- setStatus(AlarmRequest::Fail, AlarmsAdapter::OrganizerError + code);
79+ setStatus(opCode, AlarmRequest::Fail, AlarmsAdapter::OrganizerError + code);
80 } else {
81- switch (m_request->type()) {
82- case QOrganizerAbstractRequest::ItemSaveRequest: {
83+ switch (opCode) {
84+ case AlarmRequest::Saving: {
85 completeUpdate();
86 break;
87 }
88- case QOrganizerAbstractRequest::ItemRemoveRequest: {
89+ case AlarmRequest::Canceling: {
90 completeRemove();
91 break;
92 }
93- case QOrganizerAbstractRequest::ItemFetchRequest: {
94+ case AlarmRequest::Fetching: {
95 completeFetch();
96 break;
97 }
98 default:
99 qWarning() << "Unhandled request:" << m_request->type();
100- setStatus(AlarmRequest::Fail, AlarmsAdapter::UnhandledRequest);
101+ setStatus(opCode, AlarmRequest::Fail, AlarmsAdapter::UnhandledRequest);
102 break;
103 }
104
105- setStatus(AlarmRequest::Ready);
106+ setStatus(opCode, AlarmRequest::Ready);
107 }
108 break;
109 }
110 default: {
111 qWarning() << "Invalid status" << state;
112- setStatus(AlarmRequest::Fail, UCAlarm::InvalidEvent);
113+ setStatus(opCode, AlarmRequest::Fail, UCAlarm::InvalidEvent);
114 break;
115 }
116 }
117@@ -515,6 +516,23 @@
118 }
119 }
120
121+AlarmRequest::Operation AlarmRequestAdapter::requestTypeToOperation()
122+{
123+ switch (m_request->type()) {
124+ case QOrganizerAbstractRequest::ItemSaveRequest: {
125+ return AlarmRequest::Saving;
126+ }
127+ case QOrganizerAbstractRequest::ItemRemoveRequest: {
128+ return AlarmRequest::Canceling;
129+ }
130+ case QOrganizerAbstractRequest::ItemFetchRequest: {
131+ return AlarmRequest::Fetching;
132+ }
133+ default:
134+ return AlarmRequest::NoOperation;
135+ }
136+}
137+
138 void AlarmRequestAdapter::completeUpdate()
139 {
140 UCAlarm *alarm = qobject_cast<UCAlarm*>(q_ptr->parent());
141
142=== modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h'
143--- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h 2013-09-12 05:27:40 +0000
144+++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h 2013-09-24 06:30:40 +0000
145@@ -49,6 +49,7 @@
146 private:
147 QOrganizerAbstractRequest *m_request;
148
149+ AlarmRequest::Operation requestTypeToOperation();
150 void completeUpdate();
151 void completeRemove();
152 void completeFetch();
153
154=== modified file 'modules/Ubuntu/Components/plugin/alarmrequest_p.cpp'
155--- modules/Ubuntu/Components/plugin/alarmrequest_p.cpp 2013-08-28 06:55:05 +0000
156+++ modules/Ubuntu/Components/plugin/alarmrequest_p.cpp 2013-09-24 06:30:40 +0000
157@@ -35,12 +35,12 @@
158 {
159 }
160
161-void AlarmRequestPrivate::setStatus(AlarmRequest::Status status, int error)
162+void AlarmRequestPrivate::setStatus(AlarmRequest::Operation operation, AlarmRequest::Status status, int error)
163 {
164 if ((this->status != status) || (this->error != error)) {
165 this->status = status;
166 this->error = error;
167- Q_EMIT q_func()->statusChanged(status, error);
168+ Q_EMIT q_func()->statusChanged(operation, status, error);
169 }
170 }
171
172
173=== modified file 'modules/Ubuntu/Components/plugin/alarmrequest_p.h'
174--- modules/Ubuntu/Components/plugin/alarmrequest_p.h 2013-09-12 05:27:40 +0000
175+++ modules/Ubuntu/Components/plugin/alarmrequest_p.h 2013-09-24 06:30:40 +0000
176@@ -32,6 +32,12 @@
177 InProgress,
178 Fail
179 };
180+ enum Operation {
181+ NoOperation,
182+ Saving,
183+ Canceling,
184+ Fetching = 100 // this is an operation which is not reflected to QML Alarm
185+ };
186
187 explicit AlarmRequest(QObject *parent = 0);
188 AlarmRequest(bool autoDelete, QObject *parent = 0);
189@@ -47,7 +53,7 @@
190 bool wait(int msec = 0);
191
192 Q_SIGNALS:
193- void statusChanged(int status, int error);
194+ void statusChanged(int operation, int status, int error);
195
196 private:
197 Q_DECLARE_PRIVATE(AlarmRequest)
198
199=== modified file 'modules/Ubuntu/Components/plugin/alarmrequest_p_p.h'
200--- modules/Ubuntu/Components/plugin/alarmrequest_p_p.h 2013-08-28 06:55:05 +0000
201+++ modules/Ubuntu/Components/plugin/alarmrequest_p_p.h 2013-09-24 06:30:40 +0000
202@@ -37,7 +37,7 @@
203 return instance->d_func();
204 }
205
206- void setStatus(AlarmRequest::Status status, int error = UCAlarm::NoError);
207+ void setStatus(AlarmRequest::Operation operation, AlarmRequest::Status status, int error = UCAlarm::NoError);
208
209 virtual bool save(AlarmData &alarm) = 0;
210 virtual bool remove(AlarmData &alarm) = 0;
211
212=== modified file 'modules/Ubuntu/Components/plugin/ucalarm.cpp'
213--- modules/Ubuntu/Components/plugin/ucalarm.cpp 2013-09-11 07:19:55 +0000
214+++ modules/Ubuntu/Components/plugin/ucalarm.cpp 2013-09-24 06:30:40 +0000
215@@ -52,12 +52,12 @@
216 if (!request) {
217 return false;
218 }
219- QObject::connect(request, SIGNAL(statusChanged(int,int)),
220- q_ptr, SLOT(_q_syncStatus(int,int)));
221+ QObject::connect(request, SIGNAL(statusChanged(int,int,int)),
222+ q_ptr, SLOT(_q_syncStatus(int,int,int)));
223 return true;
224 }
225
226-void UCAlarmPrivate::_q_syncStatus(int status, int error) {
227+void UCAlarmPrivate::_q_syncStatus(int operation, int status, int error) {
228 UCAlarm::Status alarmStatus = static_cast<UCAlarm::Status>(status);
229 if (this->status != alarmStatus || this->error != error) {
230 this->status = alarmStatus;
231@@ -80,7 +80,7 @@
232 rawData.changes = 0;
233 }
234
235- Q_EMIT q_func()->statusChanged();
236+ Q_EMIT q_func()->statusChanged(static_cast<UCAlarm::Operation>(operation));
237 Q_EMIT q_func()->errorChanged();
238 }
239 }
240@@ -280,7 +280,6 @@
241 * alarm.save();
242 * if (alarm.error != Alarm.NoError)
243 * print("Error saving alarm, code: " + alarm.error);
244- * else alarm.reset();
245 * }
246 * }
247 * }
248@@ -569,7 +568,64 @@
249
250 /*!
251 * \qmlproperty Status Alarm::status
252- * The property holds the status of the last performed operation.
253+ * The property holds the status of the last performed operation. It can take one
254+ * of the following values:
255+ * \table
256+ * \header
257+ * \li Status code
258+ * \li Value
259+ * \li Description
260+ * \row
261+ * \li Ready
262+ * \li 1
263+ * \li Specifies either that the Alarm object is ready to perform any operation
264+ * or that the previous operation has been successfully completed.
265+ * \row
266+ * \li InProgress
267+ * \li 2
268+ * \li Specifies that there is an operation pending on Alarm object.
269+ * \row
270+ * \li Fail
271+ * \li 3
272+ * \li Specifies that the last alarm operation has failed. The failure code is
273+ * set in \l error property.
274+ * \endtable
275+ *
276+ * The notification signal has a parameter specifying the \a operation the status
277+ * refers to. The operation can take the following values:
278+ * \table
279+ * \header
280+ * \li Operation code
281+ * \li Description
282+ * \row
283+ * \li NoOperation
284+ * \li There is no operation pending. This may be set when an error occured
285+ * in the alarm adapters and the operation cannot be determined.
286+ * \row
287+ * \li Saving
288+ * \li The status reported refers to an operation requested through save().
289+ * \row
290+ * \li Canceling
291+ * \li The status reported refers to an operation requested through cancel().
292+ * \row
293+ * \li Reseting
294+ * \li The status reported refers to an operation requested through reset().
295+ * \endtable
296+ *
297+ * For example an implementation which resets the alarm data whenever
298+ * the save or cancel operations succeed would look as follows:
299+ *
300+ * \qml
301+ * Alarm {
302+ * onStatusChanged: {
303+ * if (status !== Alarm.Ready)
304+ * return;
305+ * if ((operation > Alarm.NoOperation) && (operation < Alarm.Reseting)) {
306+ * reset();
307+ * }
308+ * }
309+ * }
310+ * \endqml
311 */
312 UCAlarm::Status UCAlarm::status() const
313 {
314@@ -595,6 +651,8 @@
315 * \li - the \l daysOfWeek property is set to multiple days for one time alarm
316 * \endlist
317 *
318+ * The operation is asynchronous, and its status is reported through the \l status
319+ * property. Further operations should wait till the previous operation is completed.
320 * The operation result is stored in the \l error property.
321 */
322 void UCAlarm::save()
323@@ -610,7 +668,7 @@
324
325 UCAlarm::Error result = d->checkAlarm();
326 if (result != UCAlarm::NoError) {
327- d->_q_syncStatus(Fail, result);
328+ d->_q_syncStatus(Saving, Fail, result);
329 } else {
330 if (d->createRequest()) {
331 d->request->save(d->rawData);
332@@ -623,6 +681,8 @@
333 * The function removes the alarm from the collection. The function will fail
334 * for alarms which are not yet registered to the collection.
335 *
336+ * The operation is asynchronous, and its status is reported through the \l status
337+ * property. Further operations should wait till the previous operation is completed.
338 * The operation result is stored in the \l error property.
339 */
340 void UCAlarm::cancel()
341@@ -644,19 +704,19 @@
342 * \qmlmethod Alarm::reset()
343 * The function resets the alarm properties to its defaults. After this call the
344 * object can be used to create a new alarm event.
345+ *
346+ * \b Note: do not call this function on alarm objects retrieved from AlarmModel, as
347+ * calling it will result in the model being out of sync from the alarm database.
348 */
349 void UCAlarm::reset()
350 {
351 Q_D(UCAlarm);
352- d->error = NoError;
353- d->status = InProgress;
354+ d->_q_syncStatus(Reseting, InProgress, NoError);
355
356- delete d->request;
357- d->request = 0;
358 d->rawData = AlarmData();
359 d->setDefaults();
360 d->rawData.changes = AlarmData::AllFields;
361- d->_q_syncStatus(Ready, NoError);
362+ d->_q_syncStatus(Reseting, Ready, NoError);
363 }
364
365 #include "moc_ucalarm.cpp"
366
367=== modified file 'modules/Ubuntu/Components/plugin/ucalarm.h'
368--- modules/Ubuntu/Components/plugin/ucalarm.h 2013-08-26 10:21:25 +0000
369+++ modules/Ubuntu/Components/plugin/ucalarm.h 2013-09-24 06:30:40 +0000
370@@ -38,7 +38,7 @@
371 Q_PROPERTY(int error READ error NOTIFY errorChanged)
372 Q_PROPERTY(Status status READ status NOTIFY statusChanged)
373
374- Q_ENUMS(Status Error AlarmType DayOfWeek)
375+ Q_ENUMS(Status Operation Error AlarmType DayOfWeek)
376 Q_FLAGS(DaysOfWeek)
377 public: // enums
378 enum Status { // keep values in sync with AlarmRequest::Status
379@@ -47,6 +47,13 @@
380 Fail
381 };
382
383+ enum Operation { // keep values in sync with AlarmRequest::Operation
384+ NoOperation,
385+ Saving,
386+ Canceling,
387+ Reseting
388+ };
389+
390 enum Error {
391 NoError = 0,
392 InvalidDate = 1,
393@@ -108,7 +115,7 @@
394 void soundChanged();
395
396 void errorChanged();
397- void statusChanged();
398+ void statusChanged(Operation operation);
399
400 public Q_SLOTS:
401 void save();
402@@ -120,7 +127,7 @@
403 Q_DECLARE_PRIVATE(UCAlarm)
404 QScopedPointer<UCAlarmPrivate> d_ptr;
405
406- Q_PRIVATE_SLOT(d_func(), void _q_syncStatus(int status, int error))
407+ Q_PRIVATE_SLOT(d_func(), void _q_syncStatus(int operation, int status, int error))
408 };
409
410 Q_DECLARE_OPERATORS_FOR_FLAGS(UCAlarm::DaysOfWeek)
411
412=== modified file 'modules/Ubuntu/Components/plugin/ucalarm_p.h'
413--- modules/Ubuntu/Components/plugin/ucalarm_p.h 2013-08-26 10:20:11 +0000
414+++ modules/Ubuntu/Components/plugin/ucalarm_p.h 2013-09-24 06:30:40 +0000
415@@ -57,7 +57,7 @@
416 bool createRequest();
417
418 // private slots
419- void _q_syncStatus(int status, int error);
420+ void _q_syncStatus(int operation, int status, int error);
421 };
422
423 inline bool UCAlarmPrivate::isDaySet(int dayNumber, UCAlarm::DaysOfWeek days)
424
425=== modified file 'tests/resources/alarm/Alarms.qml'
426--- tests/resources/alarm/Alarms.qml 2013-09-12 05:27:40 +0000
427+++ tests/resources/alarm/Alarms.qml 2013-09-24 06:30:40 +0000
428@@ -32,7 +32,12 @@
429 Alarm {
430 id: alarm
431 onStatusChanged: {
432- print("alarm status= " + status + ", error=" + error);
433+ print("operation " + operation + ", status= " + status + ", error=" + error);
434+ if (status !== Alarm.Ready)
435+ return;
436+ if ((operation > Alarm.NoOperation) && (operation < Alarm.Reseting)) {
437+ reset();
438+ }
439 }
440 }
441

Subscribers

People subscribed via source and target branches

to status/vote changes: