From 17710741242f40c91edb28f29d6e5ef943a4c41b Mon Sep 17 00:00:00 2001 From: Romain Forlot Date: Thu, 2 Mar 2017 22:32:53 +0100 Subject: 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 --- src/can-bus.cpp | 21 +++++++++++++-------- src/can-signals.cpp | 23 +++++++++++++++++------ src/can-signals.hpp | 30 +++++++++++------------------- src/low-can-binding.cpp | 41 ++++++++++++++++++++++++++++------------- 4 files changed, 69 insertions(+), 46 deletions(-) (limited to 'src') 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 subscribed_signals_lock(get_subscribed_signals_mutex()); - std::map subscribed_signals = get_subscribed_signals(); - const auto& it_event = subscribed_signals.find(sig.genericName); + std::map& 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 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 subscribed_signals_lock(get_subscribed_signals_mutex()); - std::map 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& 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> 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 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 getSignals() +std::map& 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& getSignals() { return SIGNALS[MESSAGE_SET_ID]; } @@ -88,9 +104,4 @@ std::vector find_can_signals(const openxc_DynamicField &key) inline uint32_t get_CanSignal_id(const CanSignal& sig) { return sig.message->id; -} - -const std::map 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,18 +36,17 @@ 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 subscribed_signals; - +extern std::mutex subscribed_signals_mutex; std::mutex& get_subscribed_signals_mutex(); +/** + * @brief return the subscribed_signals map. + * + * return std::map - map of subscribed signals. + */ +extern std::map subscribed_signals; +std::map& get_subscribed_signals(); + /** * @brief The type signature for a CAN signal decoder. * @@ -135,7 +134,7 @@ typedef struct CanSignal CanSignal; /* Public: Return signals from an signals array filtered on name. */ -const std::vector getSignals(); +const std::vector& getSignals(); /* Public: Return the length of the array returned by getSignals(). */ size_t getSignalCount(); @@ -158,11 +157,4 @@ std::vector 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 - map of subscribed signals. - */ -const std::map 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& 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& 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 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& 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& 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& 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; } -- cgit 1.2.3-korg