Subject: [eman] Review of draft-ietf-eman-battery-mib-07

From: Alan Luchuk <[email-masked]>
Date: Wed, 13 Feb 2013 16:38:27 -0500 (EST)

Hello,

I'm trying to work my way through the Energy Management documents and
review the MIBs, so I started with the easiest one, the BATTERY-MIB.

Overall, it is clear that much work has been put into the all of the 
Energy Management WG documents.  Draft-ietf-eman-battery-mib-07  is 
in generally good shape.

I've organized the comments in descending order by severity.  The comments 
are in broad categories of "PROBLEMS", "ERRORS", "SPELLING ERRORS", 
"TECHNICAL COMMENTS", "CLARIFICATIONS", and finally "WORDING".  I think 
the PROBLEMS, ERRORS, and SPELLING ERRORS should be addressed, and are 
quick to fix.  It would be good to look at the TECHNICAL COMMENTS and 
CLARIFICATIONS sections and incorporate the suggestions if appropriate.
In many cases, the comments in the WORDING section are matters of personal 
preference, and can safely be ignored or discarded.

Hope these are of use to the working group.

Regards,
--Alan

    PROBLEMS
    --------

Page 10
-------

          batteryActualCharge             Unsigned64,

Unsigned64 is not a valid SMIv2 type.

Page 17, batteryActualCharge
----------------------------

       SYNTAX      Unsigned64

Unsigned64 is not a valid SMIv2 type.

Page 20, batteryAlarmLowTemperature DESCRIPTION
-----------------------------------------------

s/batteryTemperature rises above this/batteryTemperature falls below this/

Page 22, batteryCompliance DESCRIPTION
--------------------------------------

s/POWER-STATE-MIB/BATTERY-MIB/

    ERRORS
    ------

Page 8
------

       LAST-UPDATED "201106261200Z"         -- 26 june 2010

The LAST-UPDATED date does not match the comment.

Page 9
------

       REVISION "201106261200Z"         -- 26 June 2010

The REVISION date does not match the comment.

    SPELLING ERRORS
    ---------------

Page 19, batteryAlarmHighCycleCount DESCRIPTION
-----------------------------------------------

s/batteryAgingtNotification/batteryAgingNotification/
              ^

Page 25, Paragraph under batteryAlarmLowCapacity and batteryAlarmHighCycleCount
-------------------------------------------------------------------------------

s/and potentially loose data/and potentially lose data/

    TECHNICAL COMMENTS
    ------------------

Page 11, batteryIdentifier
--------------------------

The description clause for the batteryIdentifier suggests it is used like
a serial number.  Would it make sense to add an object that is the battery
model number?   The battery model number would allow the lookup of the
battery characteristics, and the battery serial number would allow the
battery to be traced if the battery is moved.

If another object is added for the battery model number, would the object
names "batteryModelNumber" and "batterySerialNumber" make better sense?
Would it make sense that the "batteryModelNumber" is an OID, or something
derived from an OID?

    CLARIFICATIONS
    --------------

Page 4, Paragraph 2
-------------------

s/motherboard to run the clock/motherboard to run the real-time clock/

Page 4, Paragraph 3, Sentences 3 and 4
--------------------------------------

   Change from:
   ------------

   This module includes managed objects for monitoring the
   batteries contained in an UPS system.  However, the information
   provided by these objects is limited and tailored the particular
   needs of UPS systems.

   To:
   ---

   The UPS MIB module includes managed objects for monitoring the
   batteries contained in an UPS system.  However, the information
   provided by the UPS MIB objects is limited and tailored the particular
   needs of UPS systems.

IMHO, this change clarifies the text a little about which MIB was intended.
When I first read this paragraph, it wasn't clear to me whether the words
"This module" meant the BATTERY-MIB or the UPS MIB.  Similarly, when I read
the words "these objects", it wasn't clear whether it meant the BATTERY-MIB
objects or the UPS MIB objects.  I had to read the paragraph a second time.

Page 12, batteryTechnology
--------------------------

What value should the batteryTechnology have when the
batteryType is capacitor(5)?

Page 12, batteryLastChargingCycleTime
-------------------------------------

The description clause starts:

           "The date and time of the last charging cycle."

Is that the starting time of the last charging cycle or the ending time?

    WORDING
    -------

Page 3, Paragraph 4
-------------------

s/certain threshold in order to inform/certain threshold to inform/

Page 4, Paragraph 1
-------------------

s/The former allows to trace a battery/The former allows tracing a battery/

Page 11, DESCRIPTION for batteryIdentifier
------------------------------------------

s/by a device at which a battery/by a device in which a battery/

s/re-installed at the same or/re-installed in the same or/

Page 25, Paragraph under batteryChargingAdminState
--------------------------------------------------

s/can e.g. be used by an attacker/can be used by an attacker/

Page 25, Paragraph under batteryAlarmLowCapacity and batteryAlarmHighCycleCount
-------------------------------------------------------------------------------

s/weak batteries are used/weak batteries being used/

s/and potentially loose data/and potentially lose data/

Page 26, Paragraph under batteryAlarmHighTemperature/batteryAlarmLowTemperature
-------------------------------------------------------------------------------

   Change text from:
   -----------------

      The result may e.g. be an
      unnecessary shutdown of a device if batteryAlarmHighTemperature is
      set to too low or damage to the device by too high temperatures if
      switched off or set to too high values or by damage to the battery
      when it e.g. is being charged.  Batteries can also be damaged e.g.
      in an attempt to charge them at too low temperatures.

   To:
   ---

      The result may be an unnecessary shutdown of a device if the
      batteryAlarmHighTemperature is set to too low.  The device or
      batteries may be damaged if the batteryAlarmHighTemperature is
      switched off or set to too high a value.  Batteries can be damaged
      if charged at too low a temperature.

Page 26, Paragraph 4
--------------------

s/even then, //

Page 26, Last Paragraph
-----------------------

s/it is RECOMMENDED to deploy SNMPv3/SNMPv3 deployment is RECOMMENDED/

Page 27, Section 7.1, "Entity MIB augmentation"
-----------------------------------------------

s/version 4 is need as soon/version 4 is needed as soon/

s/it is RECOMMENDED to deploy SNMPv3/SNMPv3 deployment is RECOMMENDED/

Page 28, Section 7.2, "Kind of entity"
--------------------------------------

s/we recommend to use/we recommend using/

s/sections needs to be updates/section needs to be updated/

Page 28, Section 7.4, "Notifications for removable batteries"
-------------------------------------------------------------

s/for added a batteries and removed/for adding and removing/

s/management system should get informed/management system should be informed/

    OTHER
    -----

Page 6:
-------

The overview of the BATTERY-MIB schema is nice!