From 4a083555e8d2dd62d438a8a42da02cd31249f321 Mon Sep 17 00:00:00 2001
From: Pim van Pelt <pim@ipng.nl>
Date: Wed, 24 Jan 2018 14:08:29 +0100
Subject: [PATCH] Clean up RPC response.

- Refactor mqtt_publish_stat() to take va_list
- Make publish_stat format its response with json_vprintf() so it is
  valid JSON.
- Call publish_stat in channel_set, the only place where the GPIO state
  changes
- Return (valid) JSON response in all RPCs
---
 include/main.h |  1 -
 include/mqtt.h |  2 +-
 src/channel.c  | 17 ++---------------
 src/mqtt.c     | 11 ++++++++++-
 src/rpc.c      | 13 +++----------
 5 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/include/main.h b/include/main.h
index e85feae..f31fe63 100644
--- a/include/main.h
+++ b/include/main.h
@@ -27,7 +27,6 @@ uint8_t channel_idx_by_gpio(int gpio);
 void channel_set(int idx, bool state);
 bool channel_get(int idx);
 int channel_get_total();
-void channel_report(int idx, char *msg, int msg_len);
 void channel_handler(int gpio, void *arg);
 
 #endif // __MAIN_H
diff --git a/include/mqtt.h b/include/mqtt.h
index d54d0bc..3335f20 100644
--- a/include/mqtt.h
+++ b/include/mqtt.h
@@ -9,6 +9,6 @@
 #define MQTT_TOPIC_BROADCAST_STAT "/mongoose/broadcast/stat"
 
 void mqtt_init();
-void mqtt_publish_stat(const char *stat, const char *msg);
+void mqtt_publish_stat(const char *stat, const char *fmt, ...);
 
 #endif // __MQTT_H
diff --git a/src/channel.c b/src/channel.c
index 6b414c1..a06c732 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -134,17 +134,6 @@ uint8_t channel_idx_by_gpio(int gpio) {
   return GPIO_INVALID;
 }
 
-void channel_report(int idx, char *msg, int msg_len) {
-  if (!msg || msg_len==0)
-    return;
-
-  if (idx<0 || idx>=channel_get_total())
-    return;
-
-  snprintf(msg, msg_len-1, "{idx: %d, button_gpio: %d, led_gpio: %d, relay_gpio: %d, relay_state: %d}",
-           idx, s_channels[idx].button_gpio, s_channels[idx].led_gpio, s_channels[idx].relay_gpio, s_channels[idx].relay_state);
-}
-
 void channel_set(int idx, bool state) {
   double now = mg_time();
 
@@ -157,6 +146,8 @@ void channel_set(int idx, bool state) {
     mgos_gpio_write(s_channels[idx].relay_gpio, state);
   if (s_channels[idx].led_gpio!=GPIO_INVALID)
     mgos_gpio_write(s_channels[idx].led_gpio, state);
+
+  mqtt_publish_stat("channel", "{\"idx\": %d, \"relay_state\": %d}", idx, channel_get(idx));
 }
 
 bool channel_get(int idx) {
@@ -168,7 +159,6 @@ bool channel_get(int idx) {
 
 void channel_handler(int gpio, void *arg) {
   uint8_t idx;
-  char msg[100];
   double now = mg_time();
   bool state;
 
@@ -187,8 +177,5 @@ void channel_handler(int gpio, void *arg) {
   state = channel_get(idx);
   channel_set(idx, !state);
 
-  channel_report(idx, msg, sizeof(msg));
-  mqtt_publish_stat("channel", msg);
-
   (void) arg;
 }
diff --git a/src/mqtt.c b/src/mqtt.c
index b871caa..830b320 100644
--- a/src/mqtt.c
+++ b/src/mqtt.c
@@ -3,6 +3,7 @@
 #include "mgos_config.h"
 #include "mqtt.h"
 #include "main.h"
+#include "frozen/frozen.h"
 
 static void mqtt_publish_broadcast_stat(const char *stat, const char *msg) {
   char topic[80];
@@ -59,12 +60,20 @@ static void mqtt_ev(struct mg_connection *nc, int ev, void *ev_data, void *user_
 }
 
 
-void mqtt_publish_stat(const char *stat, const char *msg) {
+void mqtt_publish_stat(const char *stat, const char *fmt, ...) {
   char topic[80];
+  char msg[200];
+  struct json_out out = JSON_OUT_BUF(msg, 200);
+  va_list ap;
 
   statusled_blink();
 
   snprintf(topic, sizeof(topic)-1, "%s%s/stat/%s", MQTT_TOPIC_PREFIX, mgos_sys_config_get_device_id(), stat);
+
+  va_start(ap, fmt);
+  json_vprintf(&out, fmt, ap);
+  va_end(ap);
+
   mgos_mqtt_pub((char*)topic, (char*)msg, strlen(msg), 0, false);
   LOG(LL_INFO, ("Sent topic='%s' msg='%s'", topic, msg));
 }
diff --git a/src/rpc.c b/src/rpc.c
index 7550d6e..da98add 100644
--- a/src/rpc.c
+++ b/src/rpc.c
@@ -41,7 +41,6 @@ static bool rpc_args_to_idx_and_gpio(struct mg_rpc_request_info *ri, struct mg_s
 
 static void rpc_channel_toggle_handler(struct mg_rpc_request_info *ri, void *cb_arg, struct mg_rpc_frame_info *fi, struct mg_str args) {
   uint8_t gpio;
-  char msg[100];
   int idx;
 
   rpc_log(ri, args);
@@ -50,8 +49,7 @@ static void rpc_channel_toggle_handler(struct mg_rpc_request_info *ri, void *cb_
     return;
 
   channel_handler(gpio, NULL);
-  channel_report(idx, msg, sizeof(msg));
-  mg_rpc_send_responsef(ri, msg);
+  mg_rpc_send_responsef(ri, "idx: %d, relay_state: %d", idx, channel_get(idx));
   ri = NULL;
 
   (void) ri;
@@ -62,7 +60,6 @@ static void rpc_channel_toggle_handler(struct mg_rpc_request_info *ri, void *cb_
 
 static void rpc_channel_get_handler(struct mg_rpc_request_info *ri, void *cb_arg, struct mg_rpc_frame_info *fi, struct mg_str args) {
   uint8_t gpio;
-  char msg[100];
   int idx;
 
   rpc_log(ri, args);
@@ -70,9 +67,7 @@ static void rpc_channel_get_handler(struct mg_rpc_request_info *ri, void *cb_arg
   if (!rpc_args_to_idx_and_gpio(ri, args, &idx, &gpio))
     return;
 
-  channel_report(idx, msg, sizeof(msg));
-  mqtt_publish_stat("channel", msg);
-  mg_rpc_send_responsef(ri, msg);
+  mg_rpc_send_responsef(ri, "idx: %d, relay_state: %d", idx, channel_get(idx));
   ri = NULL;
 
   (void) ri;
@@ -83,7 +78,6 @@ static void rpc_channel_get_handler(struct mg_rpc_request_info *ri, void *cb_arg
 
 static void rpc_channel_set_handler(struct mg_rpc_request_info *ri, void *cb_arg, struct mg_rpc_frame_info *fi, struct mg_str args) {
   uint8_t gpio;
-  char msg[100];
   int idx;
   int value;
 
@@ -109,8 +103,7 @@ static void rpc_channel_set_handler(struct mg_rpc_request_info *ri, void *cb_arg
   }
 
   channel_set(idx, (bool) value);
-  channel_report(idx, msg, sizeof(msg));
-  mg_rpc_send_responsef(ri, msg);
+  mg_rpc_send_responsef(ri, "idx: %d, relay_state: %d", idx, channel_get(idx));
   ri = NULL;
 
   (void) ri;