From 20c0dec8fd46f14c95618b7baed16c3c828ad530 Mon Sep 17 00:00:00 2001 From: Romain Forlot Date: Fri, 21 Apr 2017 18:20:42 +0200 Subject: [PATCH 1/1] Move all signals search functions into new signals_manager_t object Create a class from signals lookup and find standalone function and gather all find function into it. There is now only 1 function to find either CAN signals or diagnostic messages, results are returned using an ad-hoc struct containing vector of one or the other type pointers. This object also hold subscribed_signals map with events, so this class is a singleton. Change-Id: I3584c6a91201e6904edc6aeac0abfa1785bdeccc Signed-off-by: Romain Forlot --- CAN-binder/low-can-binding/can/can-bus.cpp | 27 +++--- CAN-binder/low-can-binding/configuration.cpp | 54 +---------- CAN-binder/low-can-binding/configuration.hpp | 4 - .../diagnostic/diagnostic-manager.cpp | 7 +- CAN-binder/low-can-binding/low-can-binding.cpp | 73 ++++++++------- CAN-binder/low-can-binding/utils/signals.cpp | 100 ++++++++++----------- CAN-binder/low-can-binding/utils/signals.hpp | 91 +++++++++---------- 7 files changed, 154 insertions(+), 202 deletions(-) diff --git a/CAN-binder/low-can-binding/can/can-bus.cpp b/CAN-binder/low-can-binding/can/can-bus.cpp index 5d8221c..ee6a7cf 100644 --- a/CAN-binder/low-can-binding/can/can-bus.cpp +++ b/CAN-binder/low-can-binding/can/can-bus.cpp @@ -59,19 +59,21 @@ std::map> can_bus_t::can_devices_; int can_bus_t::process_can_signals(can_message_t& can_message) { int processed_signals = 0; - std::vector signals; + struct utils::signals_found signals; openxc_DynamicField search_key, decoded_message; openxc_VehicleMessage vehicle_message; + configuration_t& conf = configuration_t::instance(); + utils::signals_manager_t& sm = utils::signals_manager_t::instance(); // First we have to found which can_signal_t it is search_key = build_DynamicField((double)can_message.get_id()); - configuration_t::instance().find_can_signals(search_key, signals); + signals = sm.find_signals(search_key); // Decoding the message ! Don't kill the messenger ! - for(auto& sig : signals) + for(auto& sig : signals.can_signals) { - std::lock_guard subscribed_signals_lock(get_subscribed_signals_mutex()); - std::map& s = get_subscribed_signals(); + std::lock_guard subscribed_signals_lock(sm.get_subscribed_signals_mutex()); + std::map& s = sm.get_subscribed_signals(); // DEBUG message to make easier debugger STL containers... //DEBUG(binder_interface, "Operator[] key char: %s, event valid? %d", sig.generic_name, afb_event_is_valid(s[sig.generic_name])); @@ -80,7 +82,7 @@ int can_bus_t::process_can_signals(can_message_t& can_message) //DEBUG(binder_interface, "Nb elt matched string: %d", (int)s.count(std::string(sig.generic_name)); if( s.find(sig->get_name()) != s.end() && afb_event_is_valid(s[sig->get_name()])) { - decoded_message = decoder_t::translateSignal(*sig, can_message, configuration_t::instance().get_can_signals()); + decoded_message = decoder_t::translateSignal(*sig, can_message, conf.get_can_signals()); openxc_SimpleMessage s_message = build_SimpleMessage(sig->get_name(), decoded_message); vehicle_message = build_VehicleMessage(s_message); @@ -91,7 +93,7 @@ int can_bus_t::process_can_signals(can_message_t& can_message) } } - DEBUG(binder_interface, "process_can_signals: %d/%d CAN signals processed.", processed_signals, (int)signals.size()); + DEBUG(binder_interface, "process_can_signals: %d/%d CAN signals processed.", processed_signals, (int)signals.can_signals.size()); return processed_signals; } @@ -107,8 +109,10 @@ int can_bus_t::process_diagnostic_signals(diagnostic_manager_t& manager, const c { int processed_signals = 0; - std::lock_guard subscribed_signals_lock(get_subscribed_signals_mutex()); - std::map& s = get_subscribed_signals(); + utils::signals_manager_t& sm = utils::signals_manager_t::instance(); + + std::lock_guard subscribed_signals_lock(sm.get_subscribed_signals_mutex()); + std::map& s = sm.get_subscribed_signals(); openxc_VehicleMessage vehicle_message = manager.find_and_decode_adr(can_message); if( (vehicle_message.has_simple_message && vehicle_message.simple_message.has_name) && @@ -164,6 +168,7 @@ void can_bus_t::can_event_push() openxc_VehicleMessage v_message; openxc_SimpleMessage s_message; json_object* jo; + utils::signals_manager_t& sm = utils::signals_manager_t::instance(); while(is_pushing_) { @@ -175,8 +180,8 @@ void can_bus_t::can_event_push() s_message = get_simple_message(v_message); { - std::lock_guard subscribed_signals_lock(get_subscribed_signals_mutex()); - std::map& s = get_subscribed_signals(); + std::lock_guard subscribed_signals_lock(sm.get_subscribed_signals_mutex()); + std::map& s = sm.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(); diff --git a/CAN-binder/low-can-binding/configuration.cpp b/CAN-binder/low-can-binding/configuration.cpp index a94809f..05d3696 100644 --- a/CAN-binder/low-can-binding/configuration.cpp +++ b/CAN-binder/low-can-binding/configuration.cpp @@ -90,10 +90,10 @@ void configuration_t::set_active_message_set(uint8_t id) diagnostic_message_t* configuration_t::get_diagnostic_message(std::string message_name) const { - std::vector found; - configuration_t::instance().find_diagnostic_messages(build_DynamicField(message_name), found); - if(! found.empty()) - return found.front(); + struct utils::signals_found found; + found = utils::signals_manager_t::instance().find_signals(build_DynamicField(message_name)); + if(! found.diagnostic_messages.empty()) + return found.diagnostic_messages.front(); return nullptr; } @@ -104,49 +104,3 @@ DiagnosticRequest* configuration_t::get_request_from_diagnostic_message(std::str return new DiagnosticRequest(diag_msg->build_diagnostic_request()); return nullptr; } - -/// @brief return diagnostic messages name found searching through diagnostic messages list. -/// -/// @param[in] key - can contain numeric or string value in order to search against -/// can signals or obd2 signals name. -/// @param[out] found_signals - Vector of signals name found. -/// -void configuration_t::find_diagnostic_messages(const openxc_DynamicField &key, std::vector& found_signals) -{ - switch(key.type) - { - case openxc_DynamicField_Type::openxc_DynamicField_Type_STRING: - lookup_signals_by_name(key.string_value, diagnostic_messages_[active_message_set_], found_signals); - break; - case openxc_DynamicField_Type::openxc_DynamicField_Type_NUM: - lookup_signals_by_id(key.numeric_value, diagnostic_messages_[active_message_set_], found_signals); - break; - default: - ERROR(binder_interface, "find_diagnostic_messages: wrong openxc_DynamicField specified. Use openxc_DynamicField_Type_NUM or openxc_DynamicField_Type_STRING type only."); - break; - } - DEBUG(binder_interface, "find_diagnostic_messages: Found %d signal(s)", (int)found_signals.size()); -} - -/// @brief return signals name found searching through CAN signals list. -/// -/// @param[in] key - can contain numeric or string value in order to search against -/// can signals or obd2 signals name. -/// @param[out] found_signals - provided vector to fill with pointer to matched signals. -/// -void configuration_t::find_can_signals(const openxc_DynamicField& key, std::vector& found_signals) -{ - switch(key.type) - { - case openxc_DynamicField_Type::openxc_DynamicField_Type_STRING: - lookup_signals_by_name(std::string(key.string_value), can_signals_[active_message_set_], found_signals); - break; - case openxc_DynamicField_Type::openxc_DynamicField_Type_NUM: - lookup_signals_by_id(key.numeric_value, can_signals_[active_message_set_], found_signals); - break; - default: - ERROR(binder_interface, "find_can_signals: wrong openxc_DynamicField specified. Use openxc_DynamicField_Type_NUM or openxc_DynamicField_Type_STRING type only."); - break; - } - DEBUG(binder_interface, "find_can_signals: Found %d signal(s)", (int)found_signals.size()); -} diff --git a/CAN-binder/low-can-binding/configuration.hpp b/CAN-binder/low-can-binding/configuration.hpp index cbb5ac2..c0a75e9 100644 --- a/CAN-binder/low-can-binding/configuration.hpp +++ b/CAN-binder/low-can-binding/configuration.hpp @@ -84,12 +84,8 @@ class configuration_t void set_active_message_set(uint8_t id); - void find_diagnostic_messages(const openxc_DynamicField &key, std::vector& found_signals); diagnostic_message_t* get_diagnostic_message(std::string message_name) const; DiagnosticRequest* get_request_from_diagnostic_message(std::string message_name) const; - - void find_can_signals(const openxc_DynamicField &key, std::vector& found_signals); - /* /// TODO: implement this function as method into can_bus class /// @brief Pre initialize actions made before CAN bus initialization diff --git a/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp b/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp index e3a78d0..7bbd780 100644 --- a/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp +++ b/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp @@ -21,6 +21,7 @@ #include "diagnostic-manager.hpp" #include "../utils/openxc-utils.hpp" +#include "../utils/signals.hpp" #include "../configuration.hpp" #define MAX_RECURRING_DIAGNOSTIC_FREQUENCY_HZ 10 @@ -534,9 +535,9 @@ openxc_VehicleMessage diagnostic_manager_t::relay_diagnostic_response(active_dia // If not success but completed then the pid isn't supported if(!response.success) { - std::vector found_signals; - configuration_t::instance().find_diagnostic_messages( build_DynamicField(adr->get_name()), found_signals ); - found_signals.front()->set_supported(false); + struct utils::signals_found found_signals; + found_signals = utils::signals_manager_t::instance().find_signals(build_DynamicField(adr->get_name())); + found_signals.diagnostic_messages.front()->set_supported(false); cleanup_request(adr, true); NOTICE(binder_interface, "relay_diagnostic_response: PID not supported or ill formed. Please unsubscribe from it. Error code : %d", response.negative_response_code); message = build_VehicleMessage(build_SimpleMessage(adr->get_name(), build_DynamicField("This PID isn't supported by your vehicle."))); diff --git a/CAN-binder/low-can-binding/low-can-binding.cpp b/CAN-binder/low-can-binding/low-can-binding.cpp index 6f361af..5a1dba9 100644 --- a/CAN-binder/low-can-binding/low-can-binding.cpp +++ b/CAN-binder/low-can-binding/low-can-binding.cpp @@ -88,8 +88,10 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe, { int ret; - std::lock_guard subscribed_signals_lock(get_subscribed_signals_mutex()); - std::map& s = get_subscribed_signals(); + utils::signals_manager_t& sm = utils::signals_manager_t::instance(); + + std::lock_guard subscribed_signals_lock(sm.get_subscribed_signals_mutex()); + std::map& s = sm.get_subscribed_signals(); if (s.find(sig) != s.end()) { if (!afb_event_is_valid(s[sig]) && !subscribe) @@ -107,7 +109,7 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe, { /* Event doesn't exist , so let's create it */ struct afb_event empty_event = {nullptr, nullptr}; - subscribed_signals[sig] = empty_event; + s[sig] = empty_event; ret = create_event_handle(sig, s); } @@ -129,61 +131,66 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe, /// /// @return Number of correctly subscribed signal /// -static int subscribe_unsubscribe_signals(struct afb_req request, bool subscribe, const std::vector& signals) +static int subscribe_unsubscribe_signals(struct afb_req request, bool subscribe, const struct utils::signals_found& signals) { - int rets = 0; + int ret, rets = 0; //TODO: Implement way to dynamically call the right function no matter // how much signals types we have. + configuration_t& conf = configuration_t::instance(); - for(const std::string& sig : signals) + for(const auto& sig : signals.diagnostic_messages) { - int ret; - if (active_diagnostic_request_t::is_diagnostic_signal(sig)) + DiagnosticRequest* diag_req = conf.get_request_from_diagnostic_message(sig->get_name()); + + // If the requested diagnostic message isn't supported by the car then unssubcribe. + // no matter what we want, worse case will be a fail unsubscription but at least we don't + // poll a PID for nothing. + if(sig->get_supported() && subscribe) { - DiagnosticRequest* diag_req = configuration_t::instance().get_request_from_diagnostic_message(sig); - - // If the requested diagnostic message isn't supported by the car then unssubcribe. - // no matter what we want, worse case will be a fail unsubscription but at least we don't - // poll a PID for nothing. - if(diag_req != nullptr && subscribe) - { - float frequency = diag_msg->get_frequency(); - subscribe = configuration_t::instance().get_diagnostic_manager().add_recurring_request( - diag_req, sig.c_str(), false, diag_msg->get_decoder(), diag_msg->get_callback(), (float)frequency); - //TODO: Adding callback requesting ignition status: diag_req, sig.c_str(), false, diagnostic_message_t::decode_obd2_response, diagnostic_message_t::check_ignition_status, frequency); - } - else - { - configuration_t::instance().get_diagnostic_manager().cleanup_request( - configuration_t::instance().get_diagnostic_manager().find_recurring_request(diag_req), true); - WARNING(binder_interface, "Signal: %s isn't supported. Canceling operation.", sig.c_str()); - return -1; - } + float frequency = sig->get_frequency(); + subscribe = conf.get_diagnostic_manager().add_recurring_request( + diag_req, sig->get_name().c_str(), false, sig->get_decoder(), sig->get_callback(), (float)frequency); + //TODO: Adding callback requesting ignition status: diag_req, sig.c_str(), false, diagnostic_message_t::decode_obd2_response, diagnostic_message_t::check_ignition_status, frequency); + } + else + { + conf.get_diagnostic_manager().cleanup_request( + conf.get_diagnostic_manager().find_recurring_request(diag_req), true); + WARNING(binder_interface, "Signal: %s isn't supported. Canceling operation.", sig->get_name().c_str()); + return -1; } - ret = subscribe_unsubscribe_signal(request, subscribe, sig); + ret = subscribe_unsubscribe_signal(request, subscribe, sig->get_name()); if(ret <= 0) return ret; rets++; + DEBUG(binder_interface, "Signal: %s subscribed", sig->get_name().c_str()); + } - DEBUG(binder_interface, "Signal: %s subscribed", sig.c_str()); + for(const auto& sig: signals.can_signals) + { + ret = subscribe_unsubscribe_signal(request, subscribe, sig->get_name()); + if(ret <= 0) + return ret; + rets++; + DEBUG(binder_interface, "Signal: %s subscribed", sig->get_name().c_str()); } return rets; } static int subscribe_unsubscribe_name(struct afb_req request, bool subscribe, const char *name) { - std::vector signals; + struct utils::signals_found signals; int ret = 0; openxc_DynamicField search_key = build_DynamicField(std::string(name)); - signals = find_signals(search_key); - if (signals.empty()) + signals = utils::signals_manager_t::instance().find_signals(search_key); + if (signals.can_signals.empty() && signals.diagnostic_messages.empty()) ret = 0; ret = subscribe_unsubscribe_signals(request, subscribe, signals); - NOTICE(binder_interface, "Subscribed/unsubscribe correctly to %d/%d signal(s).", ret, (int)signals.size()); + NOTICE(binder_interface, "Subscribed/unsubscribe correctly to %d/%d signal(s).", ret, (int)(signals.can_signals.size() + signals.diagnostic_messages.size()) ); return ret; } diff --git a/CAN-binder/low-can-binding/utils/signals.cpp b/CAN-binder/low-can-binding/utils/signals.cpp index 2e14a98..219dfd4 100644 --- a/CAN-binder/low-can-binding/utils/signals.cpp +++ b/CAN-binder/low-can-binding/utils/signals.cpp @@ -17,60 +17,58 @@ #include "signals.hpp" -/// -/// @brief Can signal event map making access to afb_event -/// externaly to an openxc existing structure. -/// -/// Event map is making relation between can_signal_t 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. -/// To ensure that the map object isn't modified when we read it, you -/// have to set this mutex before use subscribed_signals map object. -/// -std::mutex subscribed_signals_mutex; - -std::mutex& get_subscribed_signals_mutex() +namespace utils { - return subscribed_signals_mutex; -} + /// @brief Return singleton instance of configuration object. + signals_manager_t& signals_manager_t::instance() + { + static signals_manager_t sm; + return sm; + } -std::map& get_subscribed_signals() -{ - return subscribed_signals; -} + /// @brief Return Subscribed signals map mutex. + std::mutex& signals_manager_t::get_subscribed_signals_mutex() + { + return subscribed_signals_mutex_; + } -/// -/// @fn std::vector find_signals(const openxc_DynamicField &key) -/// @brief return signals name found searching through CAN_signals and OBD2 pid -/// -/// @param[in] key : can contain numeric or string value in order to search against -/// can signals or obd2 signals name. -/// -/// @return Vector of signals name found. -/// -std::vector find_signals(const openxc_DynamicField &key) -{ - std::vector found_signals_name; + /// + /// @brief return the subscribed_signals map. + /// + /// @return Map of subscribed signals. + std::map& signals_manager_t::get_subscribed_signals() + { + return subscribed_signals_; + } - switch(key.type) + /// + /// @fn std::vector find_signals(const openxc_DynamicField &key) + /// @brief return signals name found searching through CAN_signals and OBD2 pid + /// + /// @param[in] key : can contain numeric or string value in order to search against + /// can signals or obd2 signals name. + /// + /// @return Vector of signals name found. + /// + struct signals_found signals_manager_t::find_signals(const openxc_DynamicField &key) { - case openxc_DynamicField_Type::openxc_DynamicField_Type_STRING: - lookup_signals_by_name(key.string_value, configuration_t::instance().get_can_signals(), found_signals_name); - lookup_signals_by_name(key.string_value, configuration_t::instance().get_diagnostic_messages(), found_signals_name); - break; - case openxc_DynamicField_Type::openxc_DynamicField_Type_NUM: - lookup_signals_by_id(key.numeric_value, configuration_t::instance().get_can_signals(), found_signals_name); - lookup_signals_by_id(key.numeric_value, configuration_t::instance().get_diagnostic_messages(), found_signals_name); - break; - default: - ERROR(binder_interface, "find_signals: wrong openxc_DynamicField specified. Use openxc_DynamicField_Type_NUM or openxc_DynamicField_Type_STRING type only."); - break; + struct signals_found sf; + + switch(key.type) + { + case openxc_DynamicField_Type::openxc_DynamicField_Type_STRING: + lookup_signals_by_name(key.string_value, configuration_t::instance().get_can_signals(), sf.can_signals); + lookup_signals_by_name(key.string_value, configuration_t::instance().get_diagnostic_messages(), sf.diagnostic_messages); + break; + case openxc_DynamicField_Type::openxc_DynamicField_Type_NUM: + lookup_signals_by_id(key.numeric_value, configuration_t::instance().get_can_signals(), sf.can_signals); + lookup_signals_by_id(key.numeric_value, configuration_t::instance().get_diagnostic_messages(), sf.diagnostic_messages); + break; + default: + ERROR(binder_interface, "find_signals: wrong openxc_DynamicField specified. Use openxc_DynamicField_Type_NUM or openxc_DynamicField_Type_STRING type only."); + break; + } + DEBUG(binder_interface, "find_signals: Found %d signal(s)", (int)(sf.can_signals.size() + sf.diagnostic_messages.size())); + return sf; } - DEBUG(binder_interface, "find_signals: Found %d signal(s)", (int)found_signals_name.size()); - return found_signals_name; -} +} \ No newline at end of file diff --git a/CAN-binder/low-can-binding/utils/signals.hpp b/CAN-binder/low-can-binding/utils/signals.hpp index e941756..d2e9205 100644 --- a/CAN-binder/low-can-binding/utils/signals.hpp +++ b/CAN-binder/low-can-binding/utils/signals.hpp @@ -28,63 +28,54 @@ #include "../low-can-binding.hpp" -extern std::mutex subscribed_signals_mutex; -std::mutex& get_subscribed_signals_mutex(); - -/** - * @brief return the subscribed_signals map. - * - * return Map of subscribed signals. - */ -extern std::map subscribed_signals; -std::map& get_subscribed_signals(); - -template -void lookup_signals_by_name(const std::string& key, std::vector& signals, std::vector& found_signals) +namespace utils { - for(T& s : signals) + struct signals_found { - if(::fnmatch(key.c_str(), s.get_generic_name().c_str(), FNM_CASEFOLD) == 0) - found_signals.push_back(&s); - if(::fnmatch(key.c_str(), s.get_name().c_str(), FNM_CASEFOLD) == 0) - found_signals.push_back(&s); - } -} + std::vector can_signals; + std::vector diagnostic_messages; + }; -template -void lookup_signals_by_name(const std::string& key, std::vector& signals, std::vector& found_signals_name) -{ - for(T& s : signals) + class signals_manager_t { - if(::fnmatch(key.c_str(), s.get_generic_name().c_str(), FNM_CASEFOLD) == 0) - found_signals_name.push_back(s.get_name()); - if(::fnmatch(key.c_str(), s.get_name().c_str(), FNM_CASEFOLD) == 0) - found_signals_name.push_back(s.get_name()); - } -} + private: + std::mutex subscribed_signals_mutex_; + std::map subscribed_signals_; -template -void lookup_signals_by_id(const double key, std::vector& signals, std::vector& found_signals) -{ - for(T& s : signals) - { - if(configuration_t::instance().get_signal_id(s) == key) + signals_manager_t(); ///< Private constructor to make singleton class. + + public: + static signals_manager_t& instance(); + + std::mutex& get_subscribed_signals_mutex(); + std::map& get_subscribed_signals(); + + struct signals_found find_signals(const openxc_DynamicField &key); + void find_diagnostic_messages(const openxc_DynamicField &key, std::vector& found_signals); + void find_can_signals(const openxc_DynamicField &key, std::vector& found_signals); + + template + void lookup_signals_by_name(const std::string& key, std::vector& signals, std::vector& found_signals) { - found_signals.push_back(&s); + for(T& s : signals) + { + if(::fnmatch(key.c_str(), s.get_generic_name().c_str(), FNM_CASEFOLD) == 0) + found_signals.push_back(&s); + if(::fnmatch(key.c_str(), s.get_name().c_str(), FNM_CASEFOLD) == 0) + found_signals.push_back(&s); + } } - } -} -template -void lookup_signals_by_id(const double key, std::vector& signals, std::vector& found_signals_name) -{ - for(T& s : signals) - { - if(configuration_t::instance().get_signal_id(s) == key) + template + void lookup_signals_by_id(const double key, std::vector& signals, std::vector& found_signals) { - found_signals_name.push_back(s.get_name()); + for(T& s : signals) + { + if(configuration_t::instance().get_signal_id(s) == key) + { + found_signals.push_back(&s); + } + } } - } -} - -std::vector find_signals(const openxc_DynamicField &key); + }; +} \ No newline at end of file -- 2.16.6