Code review request

[GoogleCodeExporter]

This patch to deluge removes the wiring to VoltageC() in ReprogramGuardC on 
telos platforms and instead wires directly to the MSP430 ADC -- the arbiters 
and everything are still used but a lot of other things are no longer pulled 
in.  By my measurements, this saves 2.2k when nothing else is using the ADC and 
does the same thing.  I'd like to get this into 2.1.2 because deluge tends to 
make apps pretty tight on code.

Also, ReprogramGuard* under tos/lib/net/Deluge/extra/telosb should be removed 
because they're not pulled in -- the include order pulls in the ones under 
telos instead, which is fine.

Original issue reported on code.google.com by sdh...@gmail.com on 9 Jun 2011 at 3:16

• Blocked on: #50

Attachments:

• deluge.patch

[GoogleCodeExporter]

Quick question: what happens if an application is using the ADC?

Original comment by razvanm on 9 Jun 2011 at 4:21

[GoogleCodeExporter]

Oh, everything still works and this isn't a hack -- we're just wiring down to 
the bottom of the ADC stack.

If you wire VoltageC like ReprogramGuardC does now, you end up pulling in 
Msp430InternalVoltageC, AdcReadNowClientC, AdcReadStreamClientC, AdcReadClientC 
and so on down the line  However at the end of the day you pull in 
Msp430Adc12ClientAutoRVGC, the same thing I used -- that arbitrates access to 
the ADC channel for sharing.

The only comment I would make is that it looks like my patch doesn't release 
the resource when it's done, so it should be fixed...

Original comment by sdh...@gmail.com on 9 Jun 2011 at 4:28

[GoogleCodeExporter]

Original comment by philip.l...@gmail.com on 17 Jun 2011 at 7:40

• Changed state: Accepted

[GoogleCodeExporter]

I can confirm that burn from apps/tests/deluge/Blink works fine with this 
change. The decrease in ROM I got was 2638 bytes.

Current SVN head:

    compiled BlinkAppC to build/telosb/main.exe
           41420 bytes in ROM
            1454 bytes in RAM

Current SVN head with the patch:

    compiled BlinkAppC to build/telosb/main.exe
           38782 bytes in ROM
            1406 bytes in RAM

As Stephen said, the resource for Msp430Adc12ClientAutoRVGC is never released 
by this patch and that needs to be fixed before it gets committed.

Note: I only have one telosb so I was not able to also test the burn-net script.

Original comment by raz...@musaloiu.com on 26 Jul 2011 at 8:18

[GoogleCodeExporter]

Original comment by philip.l...@gmail.com on 3 Aug 2011 at 4:16

[GoogleCodeExporter]

Not sure if the issue referred to in 50 was what I was talking about; as far as 
I knew, all that needs to be done is call Resource.release() after reading a 
sample; I don't know any bugs in Msp430Adc12ClientAutoRVGC. Here's the patch 
that does what I was thinking.  This is really only important if you *don't* 
reprogram; if you do, it triggers a reboot so no one should ever care.

Original comment by sdh...@gmail.com on 3 Aug 2011 at 9:35

Attachments:

• deluge.patch

[GoogleCodeExporter]

The latest version of the patch looks good to me. Feel free to submit it. I 
think issue 50 is is just a misunderstanding.

Original comment by razvanm on 3 Aug 2011 at 2:58

[GoogleCodeExporter]

Done!

Original comment by sdh...@gmail.com on 3 Aug 2011 at 10:44

• Changed state: Fixed