Subject: Re: [MIB-DOCTORS] [eman] MIB-Doctor Review of draft-ietf-eman-battery-mib-11

From: Juergen Quittek <[email-masked]>
Date: Thu, 27 Feb 2014 10:06:04 +0000

Dear Tom,

Thank you very much for your review. 
Please see replies in line. This email addresses all of your issues
 except for the last one which I will reply to in a separate message.
Cheers,
    Juergen

> -----Original Message-----
> From: eman [mailto:[email-masked]] On Behalf Of Thomas Nadeau
> Sent: Freitag, 24. Januar 2014 22:24
> To: [email-masked]
> Cc: MIB Doctors (E-mail); eman mailing list
> Subject: [eman] MIB-Doctor Review of draft-ietf-eman-battery-mib-11
> 
> 
> 
> 	I reviewed this draft as part of the MIB Doctor review on Friday,
> January 24, 2014.
> In general the MIB is in good shape. I have some questions/comments inline
> begin with TOM:
> 
>    General Comments on the draft:
> 
>    1) My run of id-nits produced no errors and a few warnings that can be
> ignored.
> 
>    2) In general, please go through the MIB modules and verify that the
> Integer32 values are indeed ok to be negative values or change them to
> Unsigned. For example, there are many instances where you used Integer32
> for things like watts or amps where negative values do not make sense, to
> me at least.

We use Integer32 for Current and temperatures. For temperature we do not
use Kelvin, but degree Celsius as units which may have negative values. For
current the DESCRIPTION clause explains that positive values are for charging
current and negative values are for discharging current.

     [snip]
>    --------------------------------------------------------------------
>    -- 1.1. Battery Table
>    --------------------------------------------------------------------
>    batteryTable  OBJECT-TYPE
>        SYNTAX      SEQUENCE OF BatteryEntry
>        MAX-ACCESS  not-accessible
>        STATUS      current
>        DESCRIPTION
>            "This table provides information on batteries.  It contains
>            one conceptual row per battery.
> 
> TOM: Is this a set of batteries per managed device?

Yes. For example, the notebook I am writing this email on has two batteries.

Proposal:
OLD
        "This table provides information on batteries.  It contains 
        one conceptual row per battery.  
NEW
        "This table provides information on batteries.  It contains 
        one conceptual row per battery in a managed entity
 
     [snip]
> 
>    batteryTemperature OBJECT-TYPE
>        SYNTAX      Integer32
>        UNITS       "deci-degrees Celsius"
>        MAX-ACCESS  read-only
>        STATUS      current
>        DESCRIPTION
>            "The ambient temperature at or near the battery.
> 
> TOM: Is there some spec that defines what "near" means?

No, there is not. Actual implementations vary significantly. Some sensors are directly attached to or even inside the battery, some are a few centimeters away from the battery, but (hopefully) in the flow of heated air that passed the battery for cooling. Even for sensors inside the battery, there are problems. Sometimes single cells in a multi-cell battery become very hot. Then it depends on the location of the sensor (close to the particular cell or rather far away) if this is observable from outside.

     [snip]
> 
>    batteryAlarmHighCycleCount OBJECT-TYPE
>        SYNTAX      Unsigned32
>        MAX-ACCESS  read-write
>        STATUS      current
>        DESCRIPTION
>            "This object provides the upper threshold value for object
>            batteryChargingCycleCount.  If the value of object
>            batteryChargingCycleCount rises above this threshold,
>            a battery aging alarm will be raised.  The alarm procedure
>            may include generating a batteryAgingNotification.
> 
>            A value of 0 indicates that no alarm will be raised for any
>            value of object batteryChargingCycleCount."
> 
> TOM: Is there  any sort of holding time period over which an alarm should be
> not emitted? I can imagine a case where a battery is charged, discharged,
> etc... and just goes above and below this threshold causing a lot of
> notifications to be emitted. Another option would be to add variable that
> defines X numebr of notifications per time period.

Yes, there is. We have specified this in the DESCRIPTION clauses of 
Some NOTIFICATION-TYPEs but apparently not for all where this would
Be desirable. Please see a separate email on this issue.

For the cycle count alarm above we do not see a need for this, because 
The cycle count is monotonically increasing.
 
     [snip]
> 
>    batteryTemperatureNotification NOTIFICATION-TYPE
>        OBJECTS     {
>            batteryTemperature,
>            batteryCellIdentifier
>        }
>        STATUS      current
>        DESCRIPTION
>            "This notification can be generated when the measured
>            temperature (batteryTemperature) rises above the threshold
>            defined by object batteryAlarmHighTemperature or falls
>            below the threshold defined by object
>            batteryAlarmLowTemperature.
> 
>            If the low or high temperature has been detected for a
>            single cell or a set of cells of the battery and not for the
>            entire battery, then object batteryCellIdentifier should be
>            set to a value that identifies the cell or set of cells.
>            Otherwise, the value of object batteryCellIdentifier should
>            be set to the empty string when this notification is
>            generated."
>        ::= { batteryNotifications 4 }
> 
> TOM: The same comment about rate limiting as above. I can see when a
> battery is failing this value could float just about and below this value causing
> numerious notifications to be issued. Have you considered such cases?

Please see separate email.

Thanks,
    Juergen

---

From: Thomas Nadeau <[email-masked]>
Date: Thu, 27 Feb 2014 06:44:19 -0500

> current and negative values are for discharging current.

TOM: OK.  Just ensure that the descriptions are clear of these boundary conditions.

>        one conceptual row per battery in a managed entity

TOM: Cool.

> No, there is not. Actual implementations vary significantly. Some sensors are directly attached to or even inside the battery, some are a few centimeters away from the battery, but (hopefully) in t…

TOM: Maybe change "near" to "within close proximity" ?

> Be desirable. Please see a separate email on this issue.

TOM: Indeed. Can you please ensure that each one has this?  Otherwise we could have interop issues. 

> The cycle count is monotonically increasing.

TOM: Again, its good to explicitly put that into the description so that interop of implementations can be made easier.