From: Piotr Kosko Date: Wed, 4 Oct 2017 07:45:45 +0000 (+0200) Subject: [Sensor] Fixed some bugs X-Git-Tag: submit/tizen_3.0/20171108.104119~10 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d6a35588244179781104673ca51e1197cabb4d53;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [Sensor] Fixed some bugs [Bug] - listener interval in case of multiple setListener calls was not updated - start/stop/setListener/unsetListener was not protected against multithread access - different behaviour in below scenarios: a.> HRMrawsensor = tizen.sensorservice.getDefaultSensor("HRM_RAW"); > HRMrawsensor.stop() undefined /// nothing happen b.> HRMrawsensor = tizen.sensorservice.getDefaultSensor("HRM_RAW"); > HRMrawsensor.start(function(s){console.log(s)}, function(s){console.log(s)}) WebAPIException {code: 9, name: "NotSupportedError", message: "None HRM LED sensor is supported."} > HRMrawsensor.stop() Uncaught WebAPIException {code: 9, name: "NotSupportedError", message: "None HRM LED sensor is supported."} [Verification] Manual and Auto TCT passed 100% (s.jastrzebsk) Change-Id: I4862823f84fc386d05eabfeb3dabb7345e9c592c Signed-off-by: Piotr Kosko Signed-off-by: Szymon Jastrzebski --- diff --git a/src/sensor/sensor_api.js b/src/sensor/sensor_api.js index 8c34e338..2450cf20 100755 --- a/src/sensor/sensor_api.js +++ b/src/sensor/sensor_api.js @@ -61,6 +61,8 @@ var SensorListener = function (type, constructor) { this.sensorType = type; this.state = SensorStates.NOT_STARTED; this.callback = undefined; + this.callbackInterval = undefined; + this.callbackBatchLatency = undefined; this.constructor = constructor; }; @@ -78,6 +80,7 @@ SensorListener.prototype.start = function (successCallback, errorCallback) { native_.call('Sensor_start', {'sensorType' : thisObject.sensorType}, function(result) { if (native_.isFailure(result)) { + thisObject.state = SensorStates.NOT_STARTED; if (!T_.isNullOrUndefined(errorCallback)) { errorCallback(native_.getErrorObject(result)); } @@ -104,8 +107,9 @@ SensorListener.prototype.stop = function () { }; SensorListener.prototype.setListener = function (successCallback, interval, batchLatency) { - if (!this.callback) { - //call platform only if there was no listener registered + if (!this.callback || this.callbackInterval != interval || + this.callbackBatchLatency != batchLatency) { + //call platform only if there was no listener registered or parameters changed var result = native_.callSync('Sensor_setChangeListener', { 'sensorType' : this.sensorType, 'interval' : interval, @@ -115,6 +119,8 @@ SensorListener.prototype.setListener = function (successCallback, interval, batc throw native_.getErrorObject(result); } } + this.callbackInterval = interval; + this.callbackBatchLatency = batchLatency; this.callback = successCallback; }; diff --git a/src/sensor/sensor_service.cc b/src/sensor/sensor_service.cc index 95ebae2b..2cd3ed6e 100644 --- a/src/sensor/sensor_service.cc +++ b/src/sensor/sensor_service.cc @@ -234,7 +234,9 @@ class SensorData { common::optional is_supported_; SensorInstance& instance_; std::mutex initialization_mutex_; + std::mutex change_listener_mutex_; bool is_change_listener_set_; + bool is_starting_; std::vector> delayed_success_callbacks_; }; @@ -248,7 +250,8 @@ SensorData::SensorData(SensorInstance& instance, sensor_type_e type_enum, const previous_event_{0, 0, 0, -FLT_MAX}, // setting dumb non-zero value to differ init value from // "good" zero values from sensor instance_(instance), - is_change_listener_set_(false) { + is_change_listener_set_(false), + is_starting_(false) { type_to_string_map.insert(std::make_pair(type_enum, name)); string_to_type_map.insert(std::make_pair(name, type_enum)); @@ -275,19 +278,23 @@ void SensorData::SensorCallback(sensor_h sensor, sensor_event_s* event, void* us LoggerD("Entered: %s", type_to_string_map[that->type()].c_str()); - if (!that->delayed_success_callbacks_.empty()) { - for_each(that->delayed_success_callbacks_.begin(), that->delayed_success_callbacks_.end(), - [](std::function& callback) { - LoggerD("Calling delayed start succcess callback"); - callback(); - }); - that->delayed_success_callbacks_.erase(that->delayed_success_callbacks_.begin(), - that->delayed_success_callbacks_.end()); - if (!that->is_change_listener_set_) { - LoggerD("unregistering temporary listener used to delay start success callback"); - int ret = sensor_listener_unset_event_cb(that->listener_); - if (SENSOR_ERROR_NONE != ret) { - LoggerE("unsetting temporary listener failed: %d", ret); + { + std::lock_guard lock(that->change_listener_mutex_); + if (!that->delayed_success_callbacks_.empty()) { + for_each(that->delayed_success_callbacks_.begin(), that->delayed_success_callbacks_.end(), + [](std::function& callback) { + LoggerD("Calling delayed start succcess callback"); + callback(); + }); + that->delayed_success_callbacks_.erase(that->delayed_success_callbacks_.begin(), + that->delayed_success_callbacks_.end()); + if (!that->is_change_listener_set_) { + // if listener was not set from JS by developer, listener need to be unregistered + LoggerD("unregistering temporary listener used to delay start success callback"); + int ret = sensor_listener_unset_event_cb(that->listener_); + if (SENSOR_ERROR_NONE != ret) { + LoggerE("unsetting temporary listener failed: %d", ret); + } } } } @@ -388,9 +395,12 @@ bool SensorData::UpdateEvent(sensor_event_s* event) { PlatformResult SensorData::AddDelayedStartSuccessCb(const std::function& successCb) { LoggerD("Entered"); - delayed_success_callbacks_.push_back(successCb); - if (!is_change_listener_set_) { + delayed_success_callbacks_.push_back(successCb); + if (!is_change_listener_set_ && !is_starting_) { + // start method callback should be delayed to ensure that after triggering it, the actual + // data would be available on sensor, thus if listener is not registered yet, there is a need + // to register temporary change listener and call success callback on first change. LoggerD("Adding temporary listener by hand"); int ret = sensor_listener_set_event_cb(listener_, 10, // as small interval as possible for tmp listener @@ -421,18 +431,29 @@ PlatformResult SensorData::Start( ReportSuccess(result->get()); report_result(result); }; - res = AddDelayedStartSuccessCb(delayed_success_callback); - if (!res) { - return res; - } - sensor_listener_set_option(listener_, SENSOR_OPTION_ALWAYS_ON); - int ret = sensor_listener_start(listener_); - if (SENSOR_ERROR_NONE != ret) { - LoggerE("sensor_listener_start : %d", ret); - return GetSensorPlatformResult(ret, "sensor_listener_start"); - } + { + std::lock_guard lock(change_listener_mutex_); + res = AddDelayedStartSuccessCb(delayed_success_callback); + if (!res) { + return res; + } + if (!is_starting_) { + ScopeLogger("First attempt to start, starting listener on native level"); + sensor_listener_set_option(listener_, SENSOR_OPTION_ALWAYS_ON); + int ret = sensor_listener_start(listener_); + if (SENSOR_ERROR_NONE != ret) { + LoggerE("sensor_listener_start : %d", ret); + // removing delayed callback - listener starting failed + delayed_success_callbacks_.pop_back(); + return GetSensorPlatformResult(ret, "sensor_listener_start"); + } + is_starting_ = true; + } else { + LoggerD("Not calling native API, only delayed callback was added"); + } + } return PlatformResult(ErrorCode::NO_ERROR); } @@ -446,17 +467,21 @@ PlatformResult SensorData::Stop() { return res; } - int ret = sensor_listener_stop(listener_); - if (SENSOR_ERROR_NONE != ret) { - LoggerE("sensor_listener_stop : %d", ret); - return GetSensorPlatformResult(ret, "sensor_listener_stop"); - } + { + std::lock_guard lock(change_listener_mutex_); + int ret = sensor_listener_stop(listener_); + if (SENSOR_ERROR_NONE != ret) { + LoggerE("sensor_listener_stop : %d", ret); + return GetSensorPlatformResult(ret, "sensor_listener_stop"); + } - // reseting delayed success callbacks flag and saved event values - previous_event_ = {0, 0, 0, -FLT_MAX}; // setting dumb non-zero value to differ init value from - // "good" zero values from sensor - delayed_success_callbacks_.erase(delayed_success_callbacks_.begin(), - delayed_success_callbacks_.end()); + // reseting delayed success callbacks flag and saved event values + previous_event_ = {0, 0, 0, -FLT_MAX}; // setting dumb non-zero value to differ init value from + // "good" zero values from sensor + delayed_success_callbacks_.erase(delayed_success_callbacks_.begin(), + delayed_success_callbacks_.end()); + is_starting_ = false; + } return PlatformResult(ErrorCode::NO_ERROR); } @@ -470,24 +495,25 @@ PlatformResult SensorData::SetChangeListener(unsigned int interval, unsigned int LoggerE("Sensor initialization for sensor %s failed", type_to_string_map[type_enum_].c_str()); return res; } + { + std::lock_guard lock(change_listener_mutex_); + int ret = SENSOR_ERROR_NONE; + if (batch_latency > 0) { + ret = sensor_listener_set_max_batch_latency(listener_, batch_latency); + if (SENSOR_ERROR_NONE != ret) { + LoggerE("sensor_listener_set_max_batch_latency : %d", ret); + return GetSensorPlatformResult(ret, "Unable to set batchLatency"); + } + } - int ret = SENSOR_ERROR_NONE; - if (batch_latency > 0) { - ret = sensor_listener_set_max_batch_latency(listener_, batch_latency); + ret = sensor_listener_set_event_cb(listener_, interval, SensorCallback, this); if (SENSOR_ERROR_NONE != ret) { - LoggerE("sensor_listener_set_max_batch_latency : %d", ret); - return GetSensorPlatformResult(ret, "Unable to set batchLatency"); + LoggerE("sensor_listener_set_event_cb : %d", ret); + return GetSensorPlatformResult(ret, "sensor_listener_set_event_cb"); } + is_change_listener_set_ = true; } - ret = sensor_listener_set_event_cb(listener_, interval, SensorCallback, this); - if (SENSOR_ERROR_NONE != ret) { - LoggerE("sensor_listener_set_event_cb : %d", ret); - return GetSensorPlatformResult(ret, "sensor_listener_set_event_cb"); - } - - is_change_listener_set_ = true; - return PlatformResult(ErrorCode::NO_ERROR); } @@ -501,13 +527,16 @@ PlatformResult SensorData::UnsetChangeListener() { return res; } - int ret = sensor_listener_unset_event_cb(listener_); - if (SENSOR_ERROR_NONE != ret) { - LoggerE("sensor_listener_unset_event_cb : %d", ret); - return GetSensorPlatformResult(ret, "sensor_listener_unset_event_cb"); - } + { + std::lock_guard lock(change_listener_mutex_); + int ret = sensor_listener_unset_event_cb(listener_); + if (SENSOR_ERROR_NONE != ret) { + LoggerE("sensor_listener_unset_event_cb : %d", ret); + return GetSensorPlatformResult(ret, "sensor_listener_unset_event_cb"); + } - is_change_listener_set_ = false; + is_change_listener_set_ = false; + } return PlatformResult(ErrorCode::NO_ERROR); }