Nut upsdev - Jul 2013 - bcmxcp: Patch for using command map if present to control commands added by dstate_addcmd

Hi

Inspired by the changes by Prachi Gandhi to use the command map from the 
UPS in the bcmxcp driver, I think the map should be used, if available, 
to control
all commands the driver adds using "dstate_addcmd".
The following patch is an attempt at doing so. The patch is against the 
bcmxcp branch.

Since my UPS also supports :
#define PW_UPS_ON        (unsigned char)0x89 /* UPS on command. length 
1-2 */
#define PW_UPS_ON_TIME        (unsigned char)0x91 /* Scheduled UPS on in 
n minutes. length 3-4 */
#define PW_UPS_OFF_TIME        (unsigned char)0x93 /* Scheduled UPS off 
in n minutes. length 3-4 */
it is tempting to add support for these as well. But I do not think I 
have time for that now in the summer.




 From 0b01612658c1265cb6df8563e0ab7f4d720e8b0f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alf=20H=C3=B8gemark?= <alf at i100.no>
Date: Wed, 3 Jul 2013 20:57:58 +0200
Subject: [PATCH] bcmxcp: If UPS supplies command map, use it to control what
  commands we register with dstate_addcmd. If UPS does not supply 
command map,
  we register default commands with dstate_addcmd

---
  drivers/bcmxcp.c | 39 ++++++++++++++++++++++++++++-----------
  drivers/bcmxcp.h | 11 +++++++++--
  2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/bcmxcp.c b/drivers/bcmxcp.c
index 5c61bd4..e92ebcd 100644
--- a/drivers/bcmxcp.c
+++ b/drivers/bcmxcp.c
@@ -150,7 +150,7 @@ static long int get_long(const unsigned char*);
  static float get_float(const unsigned char *data);
  static void init_meter_map(void);
  static void init_alarm_map(void);
-static void init_command_map(int size);
+static bool_t init_command_map(int size);
  static void init_config(void);
  static void init_limit(void);
  static void init_ups_meter_map(const unsigned char *map, unsigned char 
len);
@@ -625,7 +625,7 @@ void init_alarm_map()
  }

  /* Get information on UPS commands */
-void init_command_map(int size)
+bool_t init_command_map(int size)
  {
      unsigned char answer[PW_ANSWER_MAX_SIZE];
      int res, iIndex = 0, ncounter, NumComms = 0;
@@ -634,7 +634,10 @@ void init_command_map(int size)

      res = command_read_sequence(PW_COMMAND_LIST_REQ, answer);
      if (res <= 0)
+    {
          upsdebugx(2, "No command list block.");
+        return FALSE;
+    }
      else
      {
          upsdebugx(2, "Command list block supported.");
@@ -656,17 +659,28 @@ void init_command_map(int size)

                  if (answer[iIndex] == PW_INIT_BAT_TEST)
                  {
-                    dstate_addcmd("test.battery.start");
+                    dstate_addcmd("test.battery.start");
                  }
                  else if (answer[iIndex] == PW_INIT_SYS_TEST)
                  {
-                    dstate_addcmd("test.system.start");
-                }
+                    dstate_addcmd("test.system.start");
+                }
+                else if (answer[iIndex] == PW_LOAD_OFF_RESTART)
+                {
+                    dstate_addcmd("shutdown.return");
+                }
+                else if (answer[iIndex] == PW_UPS_OFF)
+                {
+                    dstate_addcmd("shutdown.stayoff");
+                }
                  iIndex++;
              }
+            return TRUE;
          }
-        else
+        else {
              upsdebugx(1, "Invalid response received from Command List 
block");
+            return FALSE;
+        }
      }
  }

@@ -1141,6 +1155,7 @@ void upsdrv_initinfo(void)
      int iRating = 0, iIndex = 0, res, len;
      int ncpu = 0, buf;
      int conf_block_len = 0, alarm_block_len = 0, cmd_list_len = 0;
+    bool_t got_cmd_list = FALSE;

      /* Init BCM/XCP alarm descriptions */
      init_alarm_map();
@@ -1299,11 +1314,13 @@ void upsdrv_initinfo(void)

      /* Get information on UPS commands */
      if (cmd_list_len)
-        init_command_map(cmd_list_len);
-
-    dstate_addcmd("shutdown.return");
-    dstate_addcmd("shutdown.stayoff");
-    dstate_addcmd("test.battery.start");
+        got_cmd_list = init_command_map(cmd_list_len);
+    /* Add default commands if we were not able to query UPS for support */
+    if(got_cmd_list == FALSE) {
+        dstate_addcmd("shutdown.return");
+        dstate_addcmd("shutdown.stayoff");
+        dstate_addcmd("test.battery.start");
+    }

      upsh.instcmd = instcmd;
      upsh.setvar = setvar;
diff --git a/drivers/bcmxcp.h b/drivers/bcmxcp.h
index 803f7e6..b741494 100644
--- a/drivers/bcmxcp.h
+++ b/drivers/bcmxcp.h
@@ -32,9 +32,9 @@
  #define PW_COMMAND_LIST_REQ    (unsigned char)0x40 /* Available 
commands. length 1 */
  #define PW_OUT_MON_BLOCK_REQ    (unsigned char)0x41 /* Outlet monitor 
request length 1 */
  #define PW_COM_CAP_REQ        (unsigned char)0x42 /* Request 
communication capabilities. length 2    */
-#define PW_UPS_TOP_DATA_REQ    (unsigned char)0x43 /* Requsest ups 
topology data requset. length 1    */
+#define PW_UPS_TOP_DATA_REQ    (unsigned char)0x43 /* Request ups 
topology data requset. length 1    */

-/* Need autorisation before this commands */
+/* Need autorisation before these commands */
  #define PW_UPS_ON        (unsigned char)0x89 /* UPS on command. length 
1-2 */
  #define PW_LOAD_OFF_RESTART    (unsigned char)0x8A /* Delayed 
LoadPowerOff & Restart command. length 2-4 */
  #define PW_UPS_OFF        (unsigned char)0x8B /* UPS off command. 
length 1-2 */
@@ -417,5 +417,12 @@ typedef struct {
      double    (*nuf)(const char *nut_value);        /* optional NUT to 
HID mapping */
  } info_lkp_t;

+/* use explicit booleans */
+#ifndef FALSE
+typedef enum ebool { FALSE, TRUE } bool_t;
+#else
+typedef int bool_t;
+#endif
+
  #endif /*_POWERWARE_H */

-- 
1.8.1.2

On Jul 3, 2013, at 3:06 PM, Alf H?gemark wrote:
> Inspired by the changes by Prachi Gandhi to use the command map from the
UPS in the bcmxcp driver, I think the map should be used, if available, to
control
> all commands the driver adds using "dstate_addcmd".
> The following patch is an attempt at doing so. The patch is against the
bcmxcp branch.

Looks good overall, but I can't seem to get the patch to apply with 'git
am'. It looks like your email client is adding a few spaces to the beginning
of lines. Could you please re-send the last two patches as attachments?

Here's an example:
http://article.gmane.org/gmane.comp.monitoring.nut.devel/6466/raw

Also, as for the content, we should probably push some of the bool_t stuff into
a common header file, since it seems to be used in several places.

Thanks,

-- 
Charles Lepple
clepple at gmail