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

[Thomas Nadeau <[e-mail]>]

On Feb 27, 2014:5:06 AM, at 5:06 AM, Juergen Quittek <[e-mail]> wrote:

> 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:[e-mail]] On Behalf Of Thomas Nadeau
>> Sent: Freitag, 24. Januar 2014 22:24
>> To: [e-mail]
>> 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.

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

> 
>> 
>     [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

TOM: Cool.

> 
>> 
>     [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.

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

>> 
>     [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.

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

> For the cycle count alarm above we do not see a need for this, because 
> 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.

> 
>> 
>     [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
>