platform-api

Merge lp:~nick-dedekind/platform-api/lp1351109.destroy-haptic-instance into lp:platform-api

  1. lp1351109.destroy-haptic-instance
  2. Merge into trunk
Proposed by Nick Dedekind
Status: Merged
Approved by: Gerry Boland
Approved revision: 313
Merged at revision: 315
Proposed branch: lp:~nick-dedekind/platform-api/lp1351109.destroy-haptic-instance
Merge into: lp:platform-api
Prerequisite: lp:~sil2100/platform-api/location-dep-revert
Diff against target: 230 lines (+69/-32)
7 files modified
debian/changelog (+6/-0)
debian/libubuntu-application-api3.symbols (+1/-0)
include/ubuntu/application/sensors/haptic.h (+11/-1)
src/ubuntu/application/common/application/sensors/service.cpp (+22/-31)
src/ubuntu/application/testbackend/test_stubs.cpp (+5/-0)
src/ubuntu/application/ubuntu_application_api.cpp (+1/-0)
tests/test_ua_sensors_real.cpp (+23/-0)
To merge this branch: bzr merge lp:~nick-dedekind/platform-api/lp1351109.destroy-haptic-instance
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
Thomas Voß (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Ricardo Mendoza Pending
Review via email: mp+273190@code.launchpad.net

This proposal supersedes a proposal from 2015-09-29.

Commit message

Added API for destroying haptic sensor instance.

Description of the change

Added API for destroying haptic sensor instance. (LP:#1351109)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:313
http://jenkins.qa.ubuntu.com/job/platform-api-ci/438/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-amd64-ci/51/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-armhf-ci/51/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-i386-ci/51/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/platform-api-ci/438/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:313
http://jenkins.qa.ubuntu.com/job/platform-api-ci/441/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-amd64-ci/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-i386-ci/54/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/platform-api-ci/441/rebuild

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

FAILED: Continuous integration, rev:313
http://jenkins.qa.ubuntu.com/job/platform-api-ci/442/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-amd64-ci/55/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-armhf-ci/55/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/platform-api-vivid-i386-ci/55/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/platform-api-ci/442/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

I'd be happy to approve this, but the purpose of the pointer "Holder" isn't clear to me. It doesn't scope the pointer, nor prevent copying, so I don't see why it's there at all.

I would like someone more familiar with it to give the yay/nay.

Revision history for this message
Thomas Voß (thomas-voss) wrote :

> I'd be happy to approve this, but the purpose of the pointer "Holder" isn't
> clear to me. It doesn't scope the pointer, nor prevent copying, so I don't see
> why it's there at all.
>
> I would like someone more familiar with it to give the yay/nay.

We used to introduce holders to manage reference counting, which we never really came to implementing, yet. With that, removing the holder and offering a matching _destroy to _new makes sense to me.

Revision history for this message
Thomas Voß (thomas-voss) wrote :

LGTM.

review: Approve
Revision history for this message
Gerry Boland (gerboland) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2015-08-05 10:50:34 +0000
3+++ debian/changelog 2015-10-02 08:26:15 +0000
4@@ -1,3 +1,9 @@
5+platform-api (3.0.1) UNRELEASED; urgency=medium
6+
7+ * Added api for destroying haptics sensor instance (LP:#1351109)
8+
9+ -- Nick Dedekind <nick.dedekind@canonical.com> Tue, 29 Sep 2015 13:20:00 +0100
10+
11 platform-api (3.0.0+15.10.20150805-0ubuntu1) wily; urgency=medium
12
13 * New rebuild forced.
14
15=== modified file 'debian/libubuntu-application-api3.symbols'
16--- debian/libubuntu-application-api3.symbols 2015-06-15 20:29:53 +0000
17+++ debian/libubuntu-application-api3.symbols 2015-10-02 08:26:15 +0000
18@@ -71,6 +71,7 @@
19 ua_sensors_accelerometer_new@Base 0.18.1daily13.06.21
20 ua_sensors_accelerometer_set_event_rate@Base 2.1.0+14.10.20140623.1
21 ua_sensors_accelerometer_set_reading_cb@Base 0.18.1daily13.06.21
22+ ua_sensors_haptic_destroy@Base 0replaceme
23 ua_sensors_haptic_disable@Base 2.0.0+14.10.20140612
24 ua_sensors_haptic_enable@Base 2.0.0+14.10.20140612
25 ua_sensors_haptic_new@Base 2.0.0+14.10.20140612
26
27=== modified file 'include/ubuntu/application/sensors/haptic.h'
28--- include/ubuntu/application/sensors/haptic.h 2014-05-28 14:00:43 +0000
29+++ include/ubuntu/application/sensors/haptic.h 2015-10-02 08:26:15 +0000
30@@ -35,14 +35,24 @@
31 typedef void UASensorsHaptic;
32
33 /**
34- * \brief Create a new object for accessing the haptics device.
35+ * \brief Create a new object for accessing the haptics device. Ownership is transfered to caller.
36 * \ingroup sensor_access
37+ * \sa ua_sensors_haptic_destroy
38 * \returns A new instance or NULL in case of errors.
39 */
40 UBUNTU_DLL_PUBLIC UASensorsHaptic*
41 ua_sensors_haptic_new();
42
43 /**
44+ * \brief Destroys the given sensor instance and releases all resources held by the instance.
45+ * \ingroup sensor_access
46+ * \param[in] sensor The instance to be destroyed.
47+ * \post All resources held by the instance are released. The result of any operation invoked on the destroyed instance are undefined.
48+ */
49+ UBUNTU_DLL_PUBLIC void
50+ ua_sensors_haptic_destroy(UASensorsHaptic* sensor);
51+
52+ /**
53 * \brief Enables the supplied haptics device.
54 * \ingroup sensor_access
55 * \returns U_STATUS_SUCCESS if successful or U_STATUS_ERROR if an error occured.
56
57=== modified file 'src/ubuntu/application/common/application/sensors/service.cpp'
58--- src/ubuntu/application/common/application/sensors/service.cpp 2014-06-27 19:03:04 +0000
59+++ src/ubuntu/application/common/application/sensors/service.cpp 2015-10-02 08:26:15 +0000
60@@ -22,22 +22,6 @@
61
62 #include <stdlib.h>
63
64-template<typename T>
65-struct Holder
66-{
67- Holder(const T&value = T()) : value(value)
68- {
69- }
70-
71- T value;
72-};
73-
74-template<typename T>
75-Holder<T>* make_holder(const T& value)
76-{
77- return new Holder<T>(value);
78-}
79-
80 namespace dbus = core::dbus;
81 namespace uas = ubuntu::application::sensors;
82
83@@ -50,18 +34,25 @@
84 auto stub_service = dbus::Service::use_service(bus, dbus::traits::Service<uas::USensorD>::interface_name());
85 auto stub = stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/usensord/haptic"));
86
87- auto holder = make_holder(new UbuntuApplicationSensorsHaptic(stub));
88- holder->value->bus_thread = std::thread{[bus](){ bus->run(); }};
89- holder->value->bus = bus;
90-
91- return make_holder(new UbuntuApplicationSensorsHaptic(stub));
92+ auto s = new UbuntuApplicationSensorsHaptic(stub);
93+ s->bus_thread = std::thread{[bus](){ bus->run(); }};
94+ s->bus = bus;
95+
96+ return s;
97+}
98+
99+void
100+ua_sensors_haptic_destroy(UASensorsHaptic* sensor)
101+{
102+ auto s = static_cast<UbuntuApplicationSensorsHaptic*>(sensor);
103+ delete s;
104 }
105
106 UStatus
107 ua_sensors_haptic_enable(UASensorsHaptic* sensor)
108 {
109- auto s = static_cast<Holder<UbuntuApplicationSensorsHaptic*>*>(sensor);
110- s->value->enabled = true;
111+ auto s = static_cast<UbuntuApplicationSensorsHaptic*>(sensor);
112+ s->enabled = true;
113
114 return U_STATUS_SUCCESS;
115 }
116@@ -69,8 +60,8 @@
117 UStatus
118 ua_sensors_haptic_disable(UASensorsHaptic* sensor)
119 {
120- auto s = static_cast<Holder<UbuntuApplicationSensorsHaptic*>*>(sensor);
121- s->value->enabled = false;
122+ auto s = static_cast<UbuntuApplicationSensorsHaptic*>(sensor);
123+ s->enabled = false;
124
125 return U_STATUS_SUCCESS;
126 }
127@@ -83,14 +74,14 @@
128 if (sensor == nullptr)
129 return U_STATUS_ERROR;
130
131- auto s = static_cast<Holder<UbuntuApplicationSensorsHaptic*>*>(sensor);
132+ auto s = static_cast<UbuntuApplicationSensorsHaptic*>(sensor);
133
134- if (s->value->enabled == false)
135+ if (s->enabled == false)
136 return U_STATUS_ERROR;
137
138 try
139 {
140- s->value->session->invoke_method_synchronously<uas::USensorD::Haptic::Vibrate, void>(duration);
141+ s->session->invoke_method_synchronously<uas::USensorD::Haptic::Vibrate, void>(duration);
142 }
143 catch (const std::runtime_error& e)
144 {
145@@ -110,16 +101,16 @@
146 if (sensor == nullptr)
147 return U_STATUS_ERROR;
148
149- auto s = static_cast<Holder<UbuntuApplicationSensorsHaptic*>*>(sensor);
150+ auto s = static_cast<UbuntuApplicationSensorsHaptic*>(sensor);
151
152- if (s->value->enabled == false)
153+ if (s->enabled == false)
154 return U_STATUS_ERROR;
155
156 std::vector<uint32_t> p_arg (pattern, pattern + MAX_PATTERN_SIZE);
157
158 try
159 {
160- s->value->session->invoke_method_synchronously<uas::USensorD::Haptic::VibratePattern, void>(p_arg, repeat);
161+ s->session->invoke_method_synchronously<uas::USensorD::Haptic::VibratePattern, void>(p_arg, repeat);
162 }
163 catch (const std::runtime_error& e)
164 {
165
166=== modified file 'src/ubuntu/application/testbackend/test_stubs.cpp'
167--- src/ubuntu/application/testbackend/test_stubs.cpp 2015-06-15 19:49:13 +0000
168+++ src/ubuntu/application/testbackend/test_stubs.cpp 2015-10-02 08:26:15 +0000
169@@ -123,6 +123,11 @@
170 return NULL;
171 }
172
173+// Sensors
174+void ua_sensors_haptic_destroy(UASensorsHaptic*)
175+{
176+}
177+
178 UStatus ua_sensors_haptic_enable(UASensorsHaptic*)
179 {
180 return U_STATUS_ERROR;
181
182=== modified file 'src/ubuntu/application/ubuntu_application_api.cpp'
183--- src/ubuntu/application/ubuntu_application_api.cpp 2015-06-15 19:26:30 +0000
184+++ src/ubuntu/application/ubuntu_application_api.cpp 2015-10-02 08:26:15 +0000
185@@ -129,6 +129,7 @@
186
187 // Haptic Sensor
188 IMPLEMENT_CTOR0(sensors, UASensorsHaptic*, ua_sensors_haptic_new);
189+IMPLEMENT_VOID_FUNCTION1(sensors, ua_sensors_haptic_destroy, UASensorsHaptic*);
190 IMPLEMENT_FUNCTION1(sensors, UStatus, ua_sensors_haptic_enable, UASensorsHaptic*);
191 IMPLEMENT_FUNCTION1(sensors, UStatus, ua_sensors_haptic_disable, UASensorsHaptic*);
192 IMPLEMENT_FUNCTION2(sensors, UStatus, ua_sensors_haptic_vibrate_once, UASensorsHaptic*, uint32_t);
193
194=== modified file 'tests/test_ua_sensors_real.cpp'
195--- tests/test_ua_sensors_real.cpp 2014-05-20 12:45:42 +0000
196+++ tests/test_ua_sensors_real.cpp 2015-10-02 08:26:15 +0000
197@@ -32,6 +32,7 @@
198 #include <ubuntu/application/sensors/event/proximity.h>
199 #include <ubuntu/application/sensors/light.h>
200 #include <ubuntu/application/sensors/event/light.h>
201+#include <ubuntu/application/sensors/haptic.h>
202
203 using namespace std;
204
205@@ -112,3 +113,25 @@
206 cerr << "no light sensor on this hardware\n";
207 }
208 })
209+
210+
211+TESTP_F(DefaultBackendTest, CreateHaptic, {
212+ // this can succeed or fail depending on whether the hardware we run this
213+ // on actually exists; but it should never crash
214+ UASensorsHaptic *s = ua_sensors_haptic_new();
215+ if (s != NULL) {
216+ cerr << "haptic sensor present on this hardware\n";
217+ // calling its functions should not crash; we can't assert much about
218+ // their actual values, though
219+ ua_sensors_haptic_enable(s);
220+ UStatus res = ua_sensors_haptic_vibrate_once(s, 10);
221+
222+ EXPECT_EQ(U_STATUS_SUCCESS, res);
223+
224+ ua_sensors_haptic_disable(s);
225+ // caller must delete the object
226+ ua_sensors_haptic_destroy(s);
227+ } else {
228+ cerr << "no haptic sensor on this hardware\n";
229+ }
230+})

Subscribers

People subscribed via source and target branches