[prev in list] [next in list] [prev in thread] [next in thread] 

List:       rockbox-dev
Subject:    Re: Powermanagement rework
From:       Bertrik Sikken <bertrik () sikken ! nl>
Date:       2008-08-24 22:19:04
Message-ID: 48B1DE58.3050406 () sikken ! nl
[Download RAW message or body]

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
[prev in list] [next in list] [prev in thread] [next in thread]