Mercurial > louis > mq > lightsd

changeset 256:b4a0f4dcee32

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Use the new timer interface on the gateway refresh loop and speed-up bulb discovery
author Louis Opter <kalessin@kalessin.fr>
date Sun, 16 Aug 2015 18:37:31 -0700
parents f0ae1e79c4b0
children bd1132c0f8cf
files add_start_stop_timer.patch expose_a_bunch_of_metadata_in_get_light_state.patch series use_timer_interface_for_the_gateway_refresh_loop.patch
diffstat 4 files changed, 243 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/add_start_stop_timer.patch	Sun Aug 16 16:55:02 2015 -0700
+++ b/add_start_stop_timer.patch	Sun Aug 16 18:37:31 2015 -0700
@@ -1,7 +1,21 @@
 # HG changeset patch
-# Parent  e9bbea56e55f41d6bf2f774208408c8f7e9395bd
+# Parent  e65d8e17050df8f3b7265c06bc44e9b32558db7f
 Add an interface to start and stop standalone timers
 
+diff --git a/core/lightsd.c b/core/lightsd.c
+--- a/core/lightsd.c
++++ b/core/lightsd.c
+@@ -65,9 +65,9 @@
+     lgtd_listen_close_all();
+     lgtd_command_pipe_close_all();
+     lgtd_client_close_all();
+-    lgtd_lifx_timer_close();
+     lgtd_lifx_broadcast_close();
+     lgtd_lifx_gateway_close_all();
++    lgtd_lifx_timer_close();
+     event_base_free(lgtd_ev_base);
+ #if LIBEVENT_VERSION_NUMBER >= 0x02010100
+     libevent_global_shutdown();
 diff --git a/lifx/timer.c b/lifx/timer.c
 --- a/lifx/timer.c
 +++ b/lifx/timer.c
@@ -34,7 +48,7 @@
      if (lgtd_lifx_timer_context.discovery_timeout_ev) {
          event_del(lgtd_lifx_timer_context.discovery_timeout_ev);
          event_free(lgtd_lifx_timer_context.discovery_timeout_ev);
-@@ -207,3 +215,55 @@
+@@ -207,3 +215,67 @@
      lgtd_lifx_timer_discovery_timeout_event_callback(-1, 0, NULL);
      lgtd_debug("starting discovery timer");
  }
@@ -51,8 +65,9 @@
 +    timer->callback(timer, timer->ctx);
 +}
 +
-+bool
-+lgtd_lifx_timer_start_timer(int ms,
++struct lgtd_lifx_timer *
++lgtd_lifx_timer_start_timer(int flags,
++                            int ms,
 +                            void (*cb)(struct lgtd_lifx_timer *,
 +                                       union lgtd_lifx_timer_ctx),
 +                            union lgtd_lifx_timer_ctx ctx)
@@ -69,15 +84,26 @@
 +    LIST_INSERT_HEAD(&lgtd_lifx_timers, timer, link);
 +
 +    struct timeval tv = LGTD_MSECS_TO_TIMEVAL(ms);
-+    timer->event = evtimer_new(lgtd_ev_base, lgtd_lifx_timer_callback, timer);
++    timer->event = event_new(
++        lgtd_ev_base,
++        -1,
++        flags & LGTD_LIFX_TIMER_PERSISTENT ? EV_PERSIST : 0,
++        lgtd_lifx_timer_callback,
++        timer
++    );
 +    if (!timer->event || evtimer_add(timer->event, &tv)) {
 +        LIST_REMOVE(timer, link);
++        if (timer->event) {
++            event_free(timer->event);
++        }
 +        free(timer);
-+        return false;
++        return NULL;
 +    }
 +
-+    event_active(timer->event, 0, 0);
-+    return true;
++    if (flags & LGTD_LIFX_TIMER_ACTIVATE_NOW) {
++        lgtd_lifx_timer_activate_timer(timer);
++    }
++    return timer;
 +}
 +
 +void
@@ -93,7 +119,7 @@
 diff --git a/lifx/timer.h b/lifx/timer.h
 --- a/lifx/timer.h
 +++ b/lifx/timer.h
-@@ -17,13 +17,36 @@
+@@ -17,13 +17,77 @@
  
  #pragma once
  
@@ -110,6 +136,7 @@
 +    LGTD_LIFX_TIMER_DEVICE_FORCE_REFRESH_MSECS = 2000
 +};
 +
++struct timeval;
 +struct event;
 +
 +union lgtd_lifx_timer_ctx {
@@ -117,6 +144,8 @@
 +    void        *as_ptr;
 +};
 +
++// NOTE: the timer utility isn't really lifx specific, we should move it back
++// to core, that will make naming suck less too:
 +struct lgtd_lifx_timer {
 +    LIST_ENTRY(lgtd_lifx_timer) link;
 +    void                        (*callback)(struct lgtd_lifx_timer *,
@@ -125,15 +154,53 @@
 +    struct event                *event;
 +};
 +LIST_HEAD(lgtd_lifx_timer_list, lgtd_lifx_timer);
++
++// Activate the timer now, in other words make the callback pending:
++static inline void
++lgtd_lifx_timer_activate_timer(struct lgtd_lifx_timer *timer)
++{
++    assert(timer);
++
++    event_active(timer->event, 0, 0);
++}
++
++// Re-schedule a non-persistent timer with the given timeout:
++static inline bool
++lgtd_lifx_timer_reschedule_timer(struct lgtd_lifx_timer *timer,
++                                 struct timeval *tv)
++{
++    assert(timer);
++    assert(tv);
++
++    return !evtimer_add(timer->event, tv);
++}
++
++static inline bool
++lgtd_lifx_timer_ispending_timer(struct lgtd_lifx_timer *timer)
++{
++    assert(timer);
++
++    return evtimer_pending(timer->event, NULL);
++}
++
++enum lgtd_lifx_timer_flags {
++    LGTD_LIFX_TIMER_DEFAULT_FLAGS = 0,
++    LGTD_LIFX_TIMER_ACTIVATE_NOW  = 1,
++    LGTD_LIFX_TIMER_PERSISTENT    = 1 << 1,
++};
  
  bool lgtd_lifx_timer_setup(void);
  void lgtd_lifx_timer_close(void);
  void lgtd_lifx_timer_start_watchdog(void);
  void lgtd_lifx_timer_start_discovery(void);
-+bool lgtd_lifx_timer_start_timer(int, // ms, & the timer is activated right away
-+                                 void (*)(struct lgtd_lifx_timer *,
-+                                          union lgtd_lifx_timer_ctx),
-+                                 union lgtd_lifx_timer_ctx);
++
++// Create a new timer and schedule it:
++struct lgtd_lifx_timer *lgtd_lifx_timer_start_timer(int,
++                                                    int, // ms, & the timer is activated right away
++                                                    void (*)(struct lgtd_lifx_timer *,
++                                                             union lgtd_lifx_timer_ctx),
++                                                    union lgtd_lifx_timer_ctx);
++// Un-schedule and free the given timer:
 +void lgtd_lifx_timer_stop_timer(struct lgtd_lifx_timer *);
 diff --git a/tests/core/daemon/test_daemon_update_proctitle.c b/tests/core/daemon/test_daemon_update_proctitle.c
 --- a/tests/core/daemon/test_daemon_update_proctitle.c
@@ -762,26 +829,30 @@
 new file mode 100644
 --- /dev/null
 +++ b/tests/lifx/mock_timer.h
-@@ -0,0 +1,23 @@
+@@ -0,0 +1,27 @@
 +#pragma once
 +
 +#include "lifx/timer.h" // to pull the union definition
 +
 +#ifndef MOCKED_LGTD_LIFX_TIMER_START_TIMER
-+bool lgtd_lifx_timer_start_timer(int ms,
-+                                 void (*cb)(struct lgtd_lifx_timer *,
-+                                            union lgtd_lifx_timer_ctx),
-+                                 union lgtd_lifx_timer_ctx ctx)
++struct lgtd_lifx_timer *
++lgtd_lifx_timer_start_timer(int flags,
++                            int ms,
++                            void (*cb)(struct lgtd_lifx_timer *,
++                                       union lgtd_lifx_timer_ctx),
++                            union lgtd_lifx_timer_ctx ctx)
 +{
++    (void)flags;
 +    (void)ms;
 +    (void)cb;
 +    (void)ctx;
-+    return true;
++    return NULL;
 +}
 +#endif
 +
 +#ifndef MOCKED_LGTD_LIFX_TIMER_START_TIMER
-+void lgtd_lifx_timer_stop_timer(struct lgtd_lifx_timer *timer)
++void
++lgtd_lifx_timer_stop_timer(struct lgtd_lifx_timer *timer)
 +{
 +    (void)timer;
 +}
--- a/expose_a_bunch_of_metadata_in_get_light_state.patch	Sun Aug 16 16:55:02 2015 -0700
+++ b/expose_a_bunch_of_metadata_in_get_light_state.patch	Sun Aug 16 18:37:31 2015 -0700
@@ -1,5 +1,5 @@
 # HG changeset patch
-# Parent  2cfd0a561bb72156459fc8a9dde5bfbb5613724a
+# Parent  7027549920d48dc2320bb80ab1b9439474f6d7e1
 
 diff --git a/core/daemon.c b/core/daemon.c
 --- a/core/daemon.c
@@ -320,8 +320,8 @@
 +    }
 +
 +    switch (product_id) {
-+    case 1:
-+    case 2:
++    case 0x1:
++    case 0x2:
 +        return "Original";
 +    case 0xa:
 +        return "White 800";
@@ -381,22 +381,26 @@
  struct lgtd_lifx_bulb *
  lgtd_lifx_bulb_get(const uint8_t *addr)
  {
-@@ -68,6 +142,14 @@
+@@ -68,6 +142,18 @@
  
      bulb->last_light_state_at = lgtd_time_monotonic_msecs();
  
 +    union lgtd_lifx_timer_ctx ctx = { .as_uint = 0 };
 +    memcpy(&ctx.as_uint, addr, LGTD_LIFX_ADDR_LENGTH);
-+    lgtd_lifx_timer_start_timer(
++    struct lgtd_lifx_timer *timer = lgtd_lifx_timer_start_timer(
++        LGTD_LIFX_TIMER_ACTIVATE_NOW|LGTD_LIFX_TIMER_PERSISTENT,
 +        LGTD_LIFX_BULB_FETCH_HARDWARE_INFO_TIMER_MSECS,
 +        lgtd_lifx_bulb_fetch_hardware_info,
 +        ctx
 +    );
++    if (!timer) {
++        lgtd_warn("can't start a timer to fetch the bulb hardware specs");
++    }
 +
      return bulb;
  }
  
-@@ -124,6 +206,7 @@
+@@ -124,6 +210,7 @@
          LGTD_STATS_ADD_AND_UPDATE_PROCTITLE(
              bulbs_powered_on, state->power == LGTD_LIFX_POWER_ON ? 1 : -1
          );
@@ -404,7 +408,7 @@
      }
  
      lgtd_lifx_gateway_update_tag_refcounts(bulb->gw, bulb->state.tags, state->tags);
-@@ -141,6 +224,7 @@
+@@ -141,6 +228,7 @@
          LGTD_STATS_ADD_AND_UPDATE_PROCTITLE(
              bulbs_powered_on, power == LGTD_LIFX_POWER_ON ? 1 : -1
          );
@@ -412,7 +416,7 @@
      }
  
      bulb->state.power = power;
-@@ -155,3 +239,57 @@
+@@ -155,3 +243,57 @@
  
      bulb->state.tags = tags;
  }
@@ -586,7 +590,7 @@
 -    int latency = gw->last_pkt_at - gw->last_req_at;
 +    int latency = LGTD_LIFX_GATEWAY_LATENCY(gw);
      if (latency < LGTD_LIFX_GATEWAY_MIN_REFRESH_INTERVAL_MSECS) {
-         if (!event_pending(gw->refresh_ev, EV_TIMEOUT, NULL)) {
+         if (!lgtd_lifx_timer_ispending_timer(gw->refresh_timer)) {
              int timeout = LGTD_LIFX_GATEWAY_MIN_REFRESH_INTERVAL_MSECS - latency;
 @@ -573,14 +569,9 @@
          LGTD_IEEE8023MACTOA(hdr->target.device_addr, addr), pkt->power
--- a/series	Sun Aug 16 16:55:02 2015 -0700
+++ b/series	Sun Aug 16 18:37:31 2015 -0700
@@ -2,5 +2,6 @@
 make_device_timeout_2_5x_force_refresh.patch
 ignore_sigpipe.patch
 add_start_stop_timer.patch
+use_timer_interface_for_the_gateway_refresh_loop.patch
 expose_a_bunch_of_metadata_in_get_light_state.patch
 add_ipv4_to_ieee8023mac.patch #+future #+linux
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/use_timer_interface_for_the_gateway_refresh_loop.patch	Sun Aug 16 18:37:31 2015 -0700
@@ -0,0 +1,138 @@
+# HG changeset patch
+# Parent  8321e57f9a3eb3935e3710c1ab68a1d08495d4cb
+Speed-up bulbs discovery
+
+By querying a gateway's bulbs as soon as it's discovered.
+
+This is the exact same optimization done earlier with the gateway
+discovery timer.
+
+This changeset also updates the gateway code to use the new timer
+interface.
+
+diff --git a/lifx/gateway.c b/lifx/gateway.c
+--- a/lifx/gateway.c
++++ b/lifx/gateway.c
+@@ -57,13 +57,12 @@
+     assert(gw);
+ 
+     LGTD_STATS_ADD_AND_UPDATE_PROCTITLE(gateways, -1);
+-    event_del(gw->refresh_ev);
++    lgtd_lifx_timer_stop_timer(gw->refresh_timer);
+     event_del(gw->write_ev);
+     if (gw->socket != -1) {
+         evutil_closesocket(gw->socket);
+         LIST_REMOVE(gw, link);
+     }
+-    event_free(gw->refresh_ev);
+     event_free(gw->write_ev);
+     evbuffer_free(gw->write_buf);
+     for (int i = 0; i != LGTD_LIFX_GATEWAY_MAX_TAGS; i++) {
+@@ -240,13 +239,12 @@
+ }
+ 
+ static void
+-lgtd_lifx_gateway_refresh_callback(evutil_socket_t socket,
+-                                   short events,
+-                                   void *ctx)
++lgtd_lifx_gateway_refresh_callback(struct lgtd_lifx_timer *timer,
++                                   union lgtd_lifx_timer_ctx ctx)
+ {
+-    (void)socket;
+-    (void)events;
+-    lgtd_lifx_gateway_send_get_all_light_state((struct lgtd_lifx_gateway *)ctx);
++    (void)timer;
++    struct lgtd_lifx_gateway *gw = ctx.as_ptr;
++    lgtd_lifx_gateway_send_get_all_light_state(gw);
+ }
+ 
+ void
+@@ -254,7 +252,7 @@
+ {
+     assert(gw);
+ 
+-    event_active(gw->refresh_ev, 0, 0);
++    lgtd_lifx_timer_activate_timer(gw->refresh_timer);
+ }
+ 
+ static struct lgtd_lifx_bulb *
+@@ -303,6 +301,7 @@
+         lgtd_warn("can't open a new socket");
+         goto error_connect;
+     }
++
+     gw->write_ev = event_new(
+         lgtd_ev_base,
+         gw->socket,
+@@ -311,9 +310,11 @@
+         gw
+     );
+     gw->write_buf = evbuffer_new();
+-    gw->refresh_ev = evtimer_new(
+-        lgtd_ev_base, lgtd_lifx_gateway_refresh_callback, gw
+-    );
++    if (!gw->write_ev || !gw->write_buf) {
++        lgtd_warn("can't allocate a new gateway bulb");
++        goto error_allocate;
++    }
++
+     memcpy(&gw->peer, peer, sizeof(gw->peer));
+     lgtd_sockaddrtoa(peer, gw->ip_addr, sizeof(gw->ip_addr));
+     gw->port = lgtd_sockaddrport(peer);
+@@ -322,13 +323,15 @@
+     gw->next_req_at = received_at;
+     gw->last_pkt_at = received_at;
+ 
+-    struct timeval refresh_interval = LGTD_MSECS_TO_TIMEVAL(
+-        LGTD_LIFX_GATEWAY_MIN_REFRESH_INTERVAL_MSECS
++    union lgtd_lifx_timer_ctx ctx = { .as_ptr = gw };
++    gw->refresh_timer = lgtd_lifx_timer_start_timer(
++        LGTD_LIFX_TIMER_ACTIVATE_NOW,
++        LGTD_LIFX_GATEWAY_MIN_REFRESH_INTERVAL_MSECS,
++        lgtd_lifx_gateway_refresh_callback,
++        ctx
+     );
+-
+-    if (!gw->write_ev || !gw->write_buf || !gw->refresh_ev
+-        || event_add(gw->refresh_ev, &refresh_interval) != 0) {
+-        lgtd_warn("can't allocate a new gateway bulb");
++    if (!gw->refresh_timer) {
++        lgtd_warn("can't allocate a new timer");
+         goto error_allocate;
+     }
+ 
+@@ -355,9 +358,6 @@
+     if (gw->write_buf) {
+         evbuffer_free(gw->write_buf);
+     }
+-    if (gw->refresh_ev) {
+-        event_free(gw->refresh_ev);
+-    }
+ error_connect:
+     evutil_closesocket(gw->socket);
+ error_socket:
+@@ -529,10 +529,10 @@
+ 
+     int latency = LGTD_LIFX_GATEWAY_LATENCY(gw);
+     if (latency < LGTD_LIFX_GATEWAY_MIN_REFRESH_INTERVAL_MSECS) {
+-        if (!event_pending(gw->refresh_ev, EV_TIMEOUT, NULL)) {
++        if (!lgtd_lifx_timer_ispending_timer(gw->refresh_timer)) {
+             int timeout = LGTD_LIFX_GATEWAY_MIN_REFRESH_INTERVAL_MSECS - latency;
+             struct timeval tv = LGTD_MSECS_TO_TIMEVAL(timeout);
+-            evtimer_add(gw->refresh_ev, &tv);
++            lgtd_lifx_timer_reschedule_timer(gw->refresh_timer, &tv);
+             lgtd_debug(
+                 "[%s]:%hu latency is %dms, scheduling next GET_LIGHT_STATE in %dms",
+                 gw->ip_addr, gw->port, latency, timeout
+diff --git a/lifx/gateway.h b/lifx/gateway.h
+--- a/lifx/gateway.h
++++ b/lifx/gateway.h
+@@ -79,7 +79,7 @@
+     struct event                    *write_ev;
+     struct evbuffer                 *write_buf;
+     bool                            pending_refresh_req;
+-    struct event                    *refresh_ev;
++    struct lgtd_lifx_timer          *refresh_timer;
+ };
+ LIST_HEAD(lgtd_lifx_gateway_list, lgtd_lifx_gateway);
+