Michael Sevakis wrote:
3) There's a huge chunk of code (charging_algorithm_big_step) that
only gets used when CHARGING_CONTROL is defined. This seems to be
some kind of -dV/dt charging algorithm.
Can't we move this to a separate file?
We could (for example) move specific charging algorithms into
separate files and put their state machines into a charging_state()
function that is called from the main power thread.
4) If CHARGING_CONTROL really just means a -dV/dt charging algorithm,
shouldn't we rename it like that?
Kind regards,
Bertrik
The big/small step portions weren't intended to imply anything but the
minute vs. 1/2 second interval operations. The work on Gigabeat S uses them
for entirely different purposes and they're contained in a taget-specific
file. I know it's not in SVN yet but the framework in place is intended for
greater purpose. Removing that would probably mean it will get reintroduced
anyway in some manner.
Ok, maybe we'll need a function like that, but I don't like to call it
"big step" or "small step" (what's next, functions called "important
thing" and "trivial thing"?)
What I'd like to see in the power management api is a single call
that keeps the target specific charging state machine going, perhaps
call it something like xxx_tick or xxx_poll.
For c200/e200 charging I could use something like that. Currently I'm
using functions charging_state and charger_inserted to keep the charging
state updated, but they are currently called from lots of other places
too, requiring me to add protection of the state machine with a mutex...
I'd rather see a function that is called exclusively from the power
management thread and let charger_inserted and charging_state just
return a value based on the charging state machine state.
I just took a look at the gigabeat and noticed the new target specific
charging method. I like the concept (except for the names :P ) and I
think the c200/e200 charging I want to add could easily fit into that
framework.
Kind regards,
Bertrik
