I've been running this patch on my dual battery system since the 6th, and have not noticed any regressions. Richard, does a new version of the patch exist at this point?<br><br>Stu Hood<br><br><br><div><span class="gmail_quote">
On 11/6/06, <b class="gmail_sendername">Stu Hood</b> <<a href="mailto:stuhood@gmail.com">stuhood@gmail.com</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Quick nitpicks:<br><ul><li>@330 - Slight indentation problem</li><li>@364 - TRUE in boolean statement</li><li>@400 - Check against some ratio of bat->last_full instead: For instance, (rate > 0.25 * last_full)? (huge/server batteries could have rate >50000)
</li><li>@923 - Broken comment</li></ul>Otherwise, brilliant!<br><br><br>Stu Hood<br><br><br><div><div><span class="e" id="q_10ebf906e6520007_1"><span class="gmail_quote">On 11/6/06, <b class="gmail_sendername">Richard Hughes
</b> <<a href="mailto:hughsient@gmail.com" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">
hughsient@gmail.com</a>> wrote:</span></span></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><span class="e" id="q_10ebf906e6520007_3">
On Mon, 2006-11-06 at 12:03 -0500, David Zeuthen wrote:
<br>> On Sun, 2006-11-05 at 22:54 +0000, Richard Hughes wrote:<br>> > Trivial fix attached, stops a weird error message:<br>> ><br>> > 22:51:02.523 [D] acpi.c:173: Current voltage is unknown, smaller than
<br>> > 501002010575r greater than design<br>> ><br>> > Okay to commit?<br>><br>> Certainly. Please commit. Thanks.<br><br>Nahh, I got hacking and other stuff started happening:<br><br> b/hald/Makefile.am | 1
<br> b/hald/linux/acpi.c | 226 +++++++----------------<br> b/hald/linux/apm.c | 7<br> b/hald/linux/pmu.c | 188 +++++++++++--------<br> b/hald/util_pm.c | 162 ----------------<br> b/hald/util_pm.h | 8<br>
b/hald/util_battery.c | 508 ++++++++++++++++++++++++++++++++++++++++++++++++++++
<br> b/hald/util_battery.h | 64 ++++++<br> 8 files changed, 770 insertions(+), 394 deletions(-)<br><br>What I've basically done is expand the existing struct:<br><br>typedef struct {<br> int last_level;<br> int last_chargeRate;
<br> time_t last_time;<br>} batteryInfo;<br><br>So that we store the per-battery state in a quick to use structure so we<br>can start doing some clever things with multiple batteries and rate<br>calculations (like exponential discharge curve fitting).
<br><br>This has been the result of a question "why does g-p-m have a GpmPower<br>class to abstract the data from HAL" when really it was HAL that needed<br>improving to do the stuff I was doing in g-p-m - and the only way to do
<br>this was to rip out the bad battery handling code, and add it to a new<br>module with proper logic, formatting and comments.<br><br>Note: this refactoring work out of acpi.c is also required when sysfs<br>starts giving us battery class objects like that OLPC and PMU now
<br>produce. The old code can only do the checks in acpi.c, and even then<br>some were inconsistent, some only done on 2nd read and some just wrong.<br>The acpi battery code has been cleaned up and made easier to read. The
<br>acpi.c code was especially *horrible* to work on before, due to the very<br>specific ordering of checks and conversions, something that is in<br>somepart my fault.<br><br>Using a private battery object alongside HAL device allows us to add
<br>private properties to a hal device that we can use for clever things<br>like rate smoothing and better time remaining calculation that can work<br>for pmu, apm, acpi and battery class drivers. You can see the start of
<br>this in the attached patch where the rate is exponentially smoothed by<br>50%. This makes the time remaining much more stable.<br><br>I moved the driver out of util_pm and into util_battery as the HalDevice<br>dep was needed, and this was the cleanest way to still let the addons
<br>use the standalone util_pm power stuff without the deps. I think it's<br>cleanest to use HalDevice, but I can add a match to a static hashtable<br>using the UDI if you want, although this seems a bit of a bodge to me.
<br>Maybe even a static hashtable might be best for multiple batteries, I<br>dunno.<br><br>Most of the logic is copy/paste from the acpi.c, although it's quite a<br>lot easier to follow as there are loads of comments.<br>
<br>This is the first version, so there might be loads of really big<br>problems, although it's been running like a champ on my laptop for 12<br>hours. PMU is converted, but untested. APM is lightly tested. ACPI works<br>
really well. This also makes my Lenovo laptop report rate correctly with<br>and without experimental BIOS from IBM.<br><br>Code review and comments welcomed.<br><br>Richard.<br><br><br><br></span></div>_______________________________________________
<br>hal mailing list<br><a href="mailto:hal@lists.freedesktop.org" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">hal@lists.freedesktop.org</a><br><a href="http://lists.freedesktop.org/mailman/listinfo/hal" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">
http://lists.freedesktop.org/mailman/listinfo/hal</a><br><br>
<br><br></blockquote></div><br>
</blockquote></div><br>