summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRomain Forlot <romain.forlot@iot.bzh>2017-03-02 22:32:53 +0100
committerRomain Forlot <romain.forlot@iot.bzh>2017-03-02 22:32:53 +0100
commit17710741242f40c91edb28f29d6e5ef943a4c41b (patch)
treef062ead4681a8428960c5eb3a5bbacce51bf90c5
parentdff88d0ddd9054add5b8f0760e8e12c27c65c8eb (diff)
Fix: multiple subscription and maintain subscribed_signals coherence
across usage. - Transmission of a reference instead of copy. - Don't use anymore iterator on subscribed_signals map Change-Id: I5e5b7b0bb8598be3bb0ec59c29418ee937ddcc9e Signed-off-by: Romain Forlot <romain.forlot@iot.bzh>
-rw-r--r--src/can-bus.cpp21
-rw-r--r--src/can-signals.cpp23
-rw-r--r--src/can-signals.hpp30
-rw-r--r--src/low-can-binding.cpp41
4 files changed, 69 insertions, 46 deletions
diff --git a/src/can-bus.cpp b/src/can-bus.cpp
index efa6e7ac..b1dcc3ad 100644
--- a/src/can-bus.cpp
+++ b/src/can-bus.cpp
@@ -75,10 +75,16 @@ void can_bus_t::can_decode_message()
{
{
std::lock_guard<std::mutex> subscribed_signals_lock(get_subscribed_signals_mutex());
- std::map<std::string, struct afb_event> subscribed_signals = get_subscribed_signals();
- const auto& it_event = subscribed_signals.find(sig.genericName);
+ std::map<std::string, struct afb_event>& s = get_subscribed_signals();
- if(it_event != subscribed_signals.end() && afb_event_is_valid(it_event->second))
+ const auto& it = s.find(sig.genericName);
+ if (it != s.end())
+ DEBUG(binder_interface, "Iterator key: %s, event valid? %d", it->first.c_str(), afb_event_is_valid(it->second));
+ DEBUG(binder_interface, "Operator[] key char: %s, event valid? %d", sig.genericName, afb_event_is_valid(s[sig.genericName]));
+ DEBUG(binder_interface, "Operator[] key string: %s, event valid? %d", sig.genericName, afb_event_is_valid(s[std::string(sig.genericName)]));
+ DEBUG(binder_interface, "Nb elt matched char: %d", (int)s.count(sig.genericName));
+ DEBUG(binder_interface, "Nb elt matched string: %d", (int)s.count(std::string(sig.genericName)));
+ if( it != s.end() && afb_event_is_valid(it->second))
{
decoded_message = decoder.translateSignal(sig, can_message, getSignals());
@@ -87,8 +93,8 @@ void can_bus_t::can_decode_message()
std::lock_guard<std::mutex> decoded_can_message_lock(decoded_can_message_mutex_);
push_new_vehicle_message(vehicle_message);
+ new_decoded_can_message_.notify_one();
}
- new_decoded_can_message_.notify_one();
}
}
}
@@ -112,13 +118,12 @@ void can_bus_t::can_event_push()
{
std::lock_guard<std::mutex> subscribed_signals_lock(get_subscribed_signals_mutex());
- std::map<std::string, struct afb_event> subscribed_signals = get_subscribed_signals();
- const auto& it_event = subscribed_signals.find(s_message.name);
- if(it_event != subscribed_signals.end() && afb_event_is_valid(it_event->second))
+ std::map<std::string, struct afb_event>& s = get_subscribed_signals();
+ if(s.find(std::string(s_message.name)) != s.end() && afb_event_is_valid(s[std::string(s_message.name)]))
{
jo = json_object_new_object();
jsonify_simple(s_message, jo);
- afb_event_push(it_event->second, jo);
+ afb_event_push(s[std::string(s_message.name)], jo);
}
}
}
diff --git a/src/can-signals.cpp b/src/can-signals.cpp
index 82ee0dea..3832c331 100644
--- a/src/can-signals.cpp
+++ b/src/can-signals.cpp
@@ -34,6 +34,16 @@ std::vector<std::vector<CanSignal>> SIGNALS = {
{{&(CAN_MESSAGES[0][0]), "can.driver_door.open", 2, 4, 1.000000, 0.000000, 0.000000, 0.000000, {10, 0, nullptr}, false, true, nullptr, 0, false, decoder_t::booleanDecoder, nullptr, false, (float)NULL}},
};
+/**
+ * @brief Can signal event map making access to afb_event
+ * externaly to an openxc existing structure.
+ *
+ * @desc Event map is making relation between CanSignal generic name
+ * and the afb_event struct used by application framework to pushed
+ * to the subscriber.
+ */
+std::map<std::string, struct afb_event> subscribed_signals;
+
/**
* @brief Mutex allowing safe manipulation on subscribed_signals map.
* @desc To ensure that the map object isn't modified when we read it, you
@@ -46,7 +56,13 @@ std::mutex& get_subscribed_signals_mutex()
return subscribed_signals_mutex;
}
-const std::vector<CanSignal> getSignals()
+std::map<std::string, struct afb_event>& get_subscribed_signals()
+{
+ DEBUG(binder_interface, "Here are the first subscribed_signals: %s", subscribed_signals.begin()->first.c_str() );
+ return subscribed_signals;
+}
+
+const std::vector<CanSignal>& getSignals()
{
return SIGNALS[MESSAGE_SET_ID];
}
@@ -88,9 +104,4 @@ std::vector<CanSignal> find_can_signals(const openxc_DynamicField &key)
inline uint32_t get_CanSignal_id(const CanSignal& sig)
{
return sig.message->id;
-}
-
-const std::map<std::string, struct afb_event> get_subscribed_signals()
-{
- return subscribed_signals;
} \ No newline at end of file
diff --git a/src/can-signals.hpp b/src/can-signals.hpp
index a6248f29..a95b3a3a 100644
--- a/src/can-signals.hpp
+++ b/src/can-signals.hpp
@@ -36,19 +36,18 @@ extern "C"
#define MESSAGE_SET_ID 0
-/**
- * @brief Can signal event map making access to afb_event
- * externaly to an openxc existing structure.
- *
- * @desc Event map is making relation between CanSignal generic name
- * and the afb_event struct used by application framework to pushed
- * to the subscriber.
- */
-static std::map<std::string, struct afb_event> subscribed_signals;
-
+extern std::mutex subscribed_signals_mutex;
std::mutex& get_subscribed_signals_mutex();
/**
+ * @brief return the subscribed_signals map.
+ *
+ * return std::map<std::string, struct afb_event> - map of subscribed signals.
+ */
+extern std::map<std::string, struct afb_event> subscribed_signals;
+std::map<std::string, struct afb_event>& get_subscribed_signals();
+
+/**
* @brief The type signature for a CAN signal decoder.
*
* @desc A SignalDecoder transforms a raw floating point CAN signal into a number,
@@ -135,7 +134,7 @@ typedef struct CanSignal CanSignal;
/* Public: Return signals from an signals array filtered on name.
*/
-const std::vector<CanSignal> getSignals();
+const std::vector<CanSignal>& getSignals();
/* Public: Return the length of the array returned by getSignals(). */
size_t getSignalCount();
@@ -158,11 +157,4 @@ std::vector<CanSignal> find_can_signals(const openxc_DynamicField &key);
*
* @return uint32_t - unsigned integer representing the arbitration id.
*/
-inline uint32_t get_CanSignal_id(const CanSignal& sig);
-
-/**
- * @brief return the subscribed_signals map.
- *
- * return std::map<std::string, struct afb_event> - map of subscribed signals.
- */
-const std::map<std::string, struct afb_event> get_subscribed_signals(); \ No newline at end of file
+inline uint32_t get_CanSignal_id(const CanSignal& sig); \ No newline at end of file
diff --git a/src/low-can-binding.cpp b/src/low-can-binding.cpp
index c5f10f9b..420ad961 100644
--- a/src/low-can-binding.cpp
+++ b/src/low-can-binding.cpp
@@ -52,10 +52,10 @@ can_bus_t *can_bus_handler;
*
*********************************************************************************/
-static int make_subscription_unsubscription(struct afb_req request, const char* sig_name, bool subscribe)
+static int make_subscription_unsubscription(struct afb_req request, const char* sig_name, std::map<std::string, struct afb_event>& s, bool subscribe)
{
/* Make the subscription or unsubscription to the event */
- if (((subscribe ? afb_req_subscribe : afb_req_unsubscribe)(request, subscribed_signals[std::string(sig_name)])) < 0)
+ if (((subscribe ? afb_req_subscribe : afb_req_unsubscribe)(request, s[std::string(sig_name)])) < 0)
{
ERROR(binder_interface, "Operation goes wrong for signal: %s", sig_name);
return 0;
@@ -64,10 +64,10 @@ static int make_subscription_unsubscription(struct afb_req request, const char*
}
-static int create_event_handle(const char* sig_name)
+static int create_event_handle(const char* sig_name, std::map<std::string, struct afb_event>& s)
{
- subscribed_signals[std::string(sig_name)] = afb_daemon_make_event(binder_interface->daemon, sig_name);
- if (!afb_event_is_valid(subscribed_signals[std::string(sig_name)]))
+ s[std::string(sig_name)] = afb_daemon_make_event(binder_interface->daemon, sig_name);
+ if (!afb_event_is_valid(s[std::string(sig_name)]))
{
ERROR(binder_interface, "Can't create an event, something goes wrong.");
return 0;
@@ -80,7 +80,8 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe,
int ret;
std::lock_guard<std::mutex> subscribed_signals_lock(get_subscribed_signals_mutex());
- if (subscribed_signals.find(sig.genericName) != subscribed_signals.end() && !afb_event_is_valid(subscribed_signals[std::string(sig.genericName)]))
+ std::map<std::string, struct afb_event>& s = get_subscribed_signals();
+ if (s.find(sig.genericName) != s.end() && !afb_event_is_valid(s[std::string(sig.genericName)]))
{
if(!subscribe)
{
@@ -89,14 +90,14 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe,
}
else
/* Event it isn't valid annymore, recreate it */
- ret = create_event_handle(sig.genericName);
+ ret = create_event_handle(sig.genericName, s);
}
else
{
/* Event don't exist , so let's create it */
struct afb_event empty_event = {nullptr, nullptr};
subscribed_signals[sig.genericName] = empty_event;
- ret = create_event_handle(sig.genericName);
+ ret = create_event_handle(sig.genericName, s);
}
/* Check whether or not the event handler has been correctly created and
@@ -104,22 +105,35 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe,
*/
if (ret <= 0)
return ret;
- return make_subscription_unsubscription(request, sig.genericName, subscribe);
+ return make_subscription_unsubscription(request, sig.genericName, s, subscribe);
}
+/**
+ * @fn static int subscribe_unsubscribe_signals(struct afb_req request, bool subscribe, const std::vector<CanSignal>& signals)
+ * @brief subscribe to all signals in the vector signals
+ *
+ * @param[in] afb_req request : contain original request use to subscribe or unsubscribe
+ * @param[in] subscribe boolean value used to chose between a subscription operation or an unsubscription
+ * @param[in] CanSignal vector with CanSignal to subscribe
+ *
+ * @return Number of correctly subscribed signal
+ */
static int subscribe_unsubscribe_signals(struct afb_req request, bool subscribe, const std::vector<CanSignal>& signals)
{
- int ret = 0;
+ int rets = 0;
for(const auto& signal_i : signals)
{
- ret = subscribe_unsubscribe_signal(request, subscribe, signal_i);
- if(ret == 0)
+ int ret = subscribe_unsubscribe_signal(request, subscribe, signal_i);
+ if(ret <= 0)
return ret;
+ rets++;
+ DEBUG(binder_interface, "Signal: %s subscribed", signal_i.genericName);
}
- return ret;
+ return rets;
}
+// TODO
static int subscribe_unsubscribe_all(struct afb_req request, bool subscribe)
{
int e = 0;
@@ -153,6 +167,7 @@ static int subscribe_unsubscribe_name(struct afb_req request, bool subscribe, co
ret = 0;
}
ret = subscribe_unsubscribe_signals(request, subscribe, sig);
+ NOTICE(binder_interface, "Subscribed correctly to %d/%d signal(s).", ret, (int)sig.size());
}
return ret;
}