[Tinyos-devel] Bug in Msp430TimerControl.nc interface

[Eric Decker]

Hi Vlado,

Reasonable approach.  Could you also wack the TEP that references this
stuff?

TEP131

eric

with configuration parameters instead of setting these with a command
each.</p>
<pre class="literal-block">
#include &quot;Msp430Timer.h&quot;

interface Msp430TimerControl
{
  async command msp430_compare_control_t getControl();
  async command bool isInterruptPending();
  async command void clearPendingInterrupt();

  async command void setControl(msp430_compare_control_t control );
  async command void setControlAsCompare();

***
  async command void setControlAsCapture(bool low_to_high);
***



On Mon, Jun 23, 2008 at 12:56 PM, Vlado Handziski <handzisk at tkn.tu-berlin.de>
wrote:

> I have modified the interface to match the implementation. This way is
> better because keeping it bool would limit the config to only capture on
> rising or on falling edges, but not on both or none. This being HAL, all
> options should be exported to the user.
>
> Vlado
>
> On Tue, May 20, 2008 at 05:55, Eric Decker <cire831 at gmail.com> wrote:
>
>> Kevin and I implemented a hardware assisted s/w uart using Timer A2 and in
>> the process of debugging I found the following problem:
>> In tos/chips/msp430/timer/Msp430TimerControl.nc setControlAsCapture is
>> defined as:
>>
>>     async command void setControlAsCapture(bool low_to_high);
>>
>> note the bool low_to_high.
>>
>> But in the actual implementation in
>> tos/chips/msp430/timer/Msp430TimerCapComP.nc
>>
>>   async command void Control.setControlAsCapture( uint8_t cm )
>>   {
>>      TxCCTLx = captureControl( cm );
>>   }
>>
>> and captureControl looks like:
>>
>>   uint16_t captureControl(uint8_t l_cm)
>>   {
>> =>  cc_t x = {
>>       cm : l_cm & 0x03,    // capture on rising edge
>>       ccis : 0,  // capture/compare input select
>>       clld : 0,  // TBCL1 loads on write to TBCCR1
>>       cap : 1,   // compare mode
>>       scs : 1,   // synchronous capture mode
>>       ccie : 0,  // capture compare interrupt enable
>>     };
>>     return CC2int(x);
>>   }
>>
>>
>> So the interface definition lies.  The compiler doesn't catch the
>> definition mismatch because bool is a uint8_t.
>>
>> I would propose:
>>
>> Index: timer/Msp430TimerCapComP.nc
>> ===================================================================
>> RCS file:
>> /cvsroot/tinyos/tinyos-2.x/tos/chips/msp430/timer/Msp430TimerCapComP.nc,v
>> retrieving revision 1.3
>> diff -u -r1.3 Msp430TimerCapComP.nc
>> --- timer/Msp430TimerCapComP.nc 16 Feb 2008 20:03:51 -0000      1.3
>> +++ timer/Msp430TimerCapComP.nc 20 May 2008 03:53:11 -0000
>> @@ -58,10 +58,10 @@
>>      return CC2int(x);
>>    }
>>
>> -  uint16_t captureControl(uint8_t l_cm)
>> +  uint16_t captureControl(bool low_to_high)
>>    {
>>      cc_t x = {
>> -      cm : l_cm & 0x03,    // capture on rising edge
>> +      cm : (low_to_high ? 1 : 2), // 0b01 gives rising, 0b10 gives
>> falling
>>        ccis : 0,  // capture/compare input select
>>        clld : 0,  // TBCL1 loads on write to TBCCR1
>>        cap : 1,   // compare mode
>>
>>
>> who supports msp430 timers these days?
>>
>> --
>> Eric B. Decker
>> Senior (over 50 :-) Researcher
>> Autonomous Systems Lab
>> Jack Baskin School of Engineering
>> UCSC
>>
>> _______________________________________________
>> Tinyos-devel mailing list
>> Tinyos-devel at millennium.berkeley.edu
>> https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-devel
>>
>>
>


-- 
Eric B. Decker
Senior (over 50 :-) Researcher
Autonomous Systems Lab
Jack Baskin School of Engineering
UCSC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://www.millennium.berkeley.edu/pipermail/tinyos-devel/attachments/20080625/2c58b01b/attachment.htm