Send diag request only if not other one with some id running

Fix: elapsed function return that do not compute elapsed_time at first time
Fix: reworked should_send() to get it work with our scheduling workflow
Reschedule trying to send request if recurring even it is in flight.

Change-Id: Idce54cb9bf62616c891a8a25a6c478f931350bc7
Signed-off-by: Romain Forlot <romain.forlot@iot.bzh>

diff --git a/src/diagnostic/active-diagnostic-request.cpp b/src/diagnostic/active-diagnostic-request.cpp
index a9c21c6..ad6e59e 100644 (file)
--- a/src/diagnostic/active-diagnostic-request.cpp
+++ b/src/diagnostic/active-diagnostic-request.cpp
@@ -155,14 +155,13 @@ bool active_diagnostic_request_t::is_diagnostic_signal(const std::string& name)
 
 /// @brief Check is the request should be sent or not
 ///
-/// @return true if the request isn't already running and not
-/// recurring nor completed, or it is recurring its clock elapsed so it's
-/// time to send another one.
+/// @return true if the request is not running or recurring nor completed,
+/// or it's recurring, its clock elapsed, request was successful
+/// so it's time to send another one.
 bool active_diagnostic_request_t::should_send()
 {
-       return !get_in_flight() && (
-                       (!get_recurring() && !request_completed()) ||
-                       (get_recurring() && get_frequency_clock().elapsed(true)));
+       return !in_flight_ || (!recurring_ && !request_completed()) ||
+                       (recurring_ && frequency_clock_.elapsed(true) && request_completed() && handle_->success);
 }
 
 /// @brief check if the timeout clock has elapsed

diff --git a/src/diagnostic/diagnostic-manager.cpp b/src/diagnostic/diagnostic-manager.cpp
index cf84ecc..151c78e 100644 (file)
--- a/src/diagnostic/diagnostic-manager.cpp
+++ b/src/diagnostic/diagnostic-manager.cpp
@@ -426,6 +426,17 @@ bool diagnostic_manager_t::clear_to_send(active_diagnostic_request_t* request) c
        return true;
 }
 
+int diagnostic_manager_t::reschedule_request(sd_event_source *s, uint64_t usec, active_diagnostic_request_t* adr)
+{
+       usec = usec + (uint64_t)(frequency_clock_t::frequency_to_period(adr->get_frequency_clock().get_frequency())*MICRO);
+       DEBUG(binder_interface, "send_request: Event loop state: %d. usec: %ld", sd_event_get_state(afb_daemon_get_event_loop(binder_interface->daemon)), usec);
+       if(sd_event_source_set_time(s, usec) >= 0)
+               if(sd_event_source_set_enabled(s, SD_EVENT_ON) >= 0)
+                       return 0;
+       sd_event_source_unref(s);
+       return -1;
+}
+
 /// @brief Systemd timer event callback use to send CAN messages at regular interval. Depending
 /// on the diagnostic message frequency.
 ///
@@ -443,38 +454,27 @@ int diagnostic_manager_t::send_request(sd_event_source *s, uint64_t usec, void *
        DiagnosticRequest* request = (DiagnosticRequest*)userdata;
        active_diagnostic_request_t* adr = dm.find_recurring_request(request);
 
-//     if(adr != nullptr && adr->get_can_bus_dev() == dm.get_can_bus_dev() && adr->should_send() &&
-//             dm.clear_to_send(adr))
-       if(adr != nullptr && adr->get_can_bus_dev() == dm.get_can_bus_dev())
+       if(adr != nullptr && adr->get_can_bus_dev() == dm.get_can_bus_dev() && adr->should_send() &&
+               dm.clear_to_send(adr))
        {
                adr->get_frequency_clock().tick();
                start_diagnostic_request(&dm.shims_, adr->get_handle());
-               if(adr->get_handle()->completed)
+               if(adr->get_handle()->completed && !adr->get_handle()->success)
                {
-                       if(!adr->get_handle()->success)
-                       {
                                ERROR(binder_interface, "send_request: Fatal error sending diagnostic request");
                                sd_event_source_unref(s);
                                return -1;
-                       }
                }
-               else
-                       WARNING(binder_interface, "send_request: There was a problem sending your request using bus %s.", adr->get_can_bus_dev()->get_device_name().c_str());
 
                adr->get_timeout_clock().tick();
                adr->set_in_flight(true);
+       }
 
-               if(adr->get_recurring())
-               {
-                       usec = usec + (uint64_t)(frequency_clock_t::frequency_to_period(adr->get_frequency_clock().get_frequency())*MICRO);
-                       DEBUG(binder_interface, "send_request: Event loop state: %d. usec: %ld", sd_event_get_state(afb_daemon_get_event_loop(binder_interface->daemon)), usec);
-                       if(sd_event_source_set_time(s, usec) >= 0)
-                               if(sd_event_source_set_enabled(s, SD_EVENT_ON) >= 0)
-                                       return 0;
-                       sd_event_source_unref(s);
-                       return -1;
-               }
+       if(adr->get_recurring())
+       {
+               return dm.reschedule_request(s, usec, adr);
        }
+
        sd_event_source_unref(s);
        ERROR(binder_interface, "send_request: Something goes wrong when submitting a new request to the CAN bus");
        return -2;

diff --git a/src/diagnostic/diagnostic-manager.hpp b/src/diagnostic/diagnostic-manager.hpp
index bd5cefb..3edb2b1 100644 (file)
--- a/src/diagnostic/diagnostic-manager.hpp
+++ b/src/diagnostic/diagnostic-manager.hpp
@@ -87,6 +87,7 @@ public:
        // Sendig requests part
        bool conflicting(active_diagnostic_request_t* request, active_diagnostic_request_t* candidate) const;
        bool clear_to_send(active_diagnostic_request_t* request) const;
+       int reschedule_request(sd_event_source *s, uint64_t usec, active_diagnostic_request_t* adr);
        static int send_request(sd_event_source *s, uint64_t usec, void *userdata);
 
        // Decoding part

diff --git a/src/utils/timer.cpp b/src/utils/timer.cpp
index 7c2560e..540cfe0 100644 (file)
--- a/src/utils/timer.cpp
+++ b/src/utils/timer.cpp
@@ -73,7 +73,7 @@ bool frequency_clock_t::started()
 
 time_function_t frequency_clock_t::get_time_function()
 {
-       return time_function_ != nullptr ? time_function_ : system_time_ms;
+       return time_function_ != nullptr ? time_function_ : system_time_us;
 }
 
 bool frequency_clock_t::elapsed(bool stagger)
@@ -82,9 +82,9 @@ bool frequency_clock_t::elapsed(bool stagger)
        float elapsed_time = 0;
        if(!started() && stagger)
                last_tick_ = get_time_function()() - (rand() % int(period));
-       else
-               // Make sure it ticks the the first call
-               elapsed_time = !started() ? period : get_time_function()() - last_tick_;
+
+       // Make sure it ticks the the first call
+       elapsed_time = !started() ? period : get_time_function()() - last_tick_;
 
        return frequency_ == 0 || elapsed_time >= period;
 }