From: Jerry Van Baren <gerald.vanbaren_at_smiths-aerospace.com>
Date: Wed, 2 Mar 2005 08:14:18 -0500

Double-quoting both Jörg and Linus...

[IDC]Dragon wrote:
> Thanks Jerry,
>
> I've downloaded your patch, it applies flawless. Will compile and just use
> it, let's see. I've only briefly looked into the code, powermgmnt.c is still
> a mess. ;-)
>
> Somehow I've hoped we can get away with something a lot more simple.

Hey, c'mon, its a simpler than it was! :-) It is smaller too :-).

I didn't check my change statistics, but I suspect it is pretty close to
100% changed with 75% re-use (a quarter of the code is new and the
remaining 3/4 was moved around and consolidated). Maybe a little higher
in the "new" category ;-). The patch of powermgmt.c is pretty nasty, it
would have been a simpler patch to just delete the old and include the new.

> For the option clutter, I think we can remove the trickle charge (making
> this always on), and perhaps the deep discharge (always off).

Yes, that would be a good improvement. The whole deep discharge vs.
trickle charge enable/disable would profit from a "step back and
rethink" approach: look again at what we want to accomplish and then
(re-)implement it cleanly.

> Are you starting with charging always? In the past, it's been a problem of
> not doing so on flat cells, causing the unit to die again right after
> powerup.

In the one minute "sleep" loop, I now check for charger inserted/removed
and abort the sleep so that the main loop runs and Does The Right
Thing[tm]. There are no (major) hidden assumptions that the main loop
runs only once per minute, so this is fine. The result is that charging
will kick on as soon as you insert the charger rather than after up to a
minute delay. This is a Good Thing because:
1) People expect that to happen and get very concerned if it doesn't
2) Flat batteries may not last the minute

> Any other developer opinions on if and how to bring this patch in? I'm in
> favour of "in dubio pro patch", bring it in, solve problems in the field, if
> any. Otherwise patches tend to age and get foul, err, don't apply cleanly
> any more.
>
> Jörg
>

Linus Nielsen Feltzing wrote:
> [IDC]Dragon wrote:
>
>> Any other developer opinions on if and how to bring this patch in?
I'm in
>> favour of "in dubio pro patch", bring it in, solve problems in the
>> field, if
>> any. Otherwise patches tend to age and get foul, err, don't apply
cleanly
>> any more.
>
>
> I agree 100%. Bring it in ASAP.
>
> I too want to have the charging code as simple as possible. I think we
> could combine the trickle/deep discharge into one.
>
> Linus

OK, I still have an assignment, but I'll negotiate for a deal :-).
Apply the patch so that I can start with a clean slate and I'll do some
more improving.

Improvement List (suggestions welcome):
---------------------------------------
* Rationalize "deep discharge" and "trickle charge" options, removing
the trickle charge option.

* Add a proportional term to the charge feedback control loop (currently
it only has an integral term). This would allow better control (less
overshoot/faster recovery), especially when doing just a top-off and
when transitioning from "full charge" to top-off to trickle.

* Always look for further simplifications.

gvb
P.S. Doing a fsync() after writing to my powermgmt.csv log fixed the
"zero length file" problem. Thanks for the tip.
_______________________________________________
http://cool.haxx.se/mailman/listinfo/rockbox
Received on 2005-03-02