[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 <p.kosko@samsung.com>
Signed-off-by: Szymon Jastrzebski <s.jastrzebsk@partner.samsung.com>

diff --git a/src/sensor/sensor_api.js b/src/sensor/sensor_api.js
index 8c34e338fceb8117859d09e4779600037aa52bcf..2450cf20c0a593694bc7df7bc7240dbd245b1e16 100755 (executable)
--- 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 95ebae2b14c8b09c6ec3345998963235d4c54f7a..2cd3ed6ef8e047b57fd51ef053e263477ac38e37 100644 (file)
--- a/src/sensor/sensor_service.cc
+++ b/src/sensor/sensor_service.cc
@@ -234,7 +234,9 @@ class SensorData {
   common::optional<bool> is_supported_;
   SensorInstance& instance_;
   std::mutex initialization_mutex_;
+  std::mutex change_listener_mutex_;
   bool is_change_listener_set_;
+  bool is_starting_;
   std::vector<std::function<void()>> 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<void()>& 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<std::mutex> 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<void()>& 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<void()>& 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<picojson::object>());
     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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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);
 }