zecke at openmoko.org
Sat Jul 26 22:19:31 CEST 2008
On Saturday 26 July 2008 20:58:26 Lorn Potter wrote:
> > My approach:
> > - I do clean up the incosistent usage of 1, 3, 4 spaces mixed with tabs
> > and placing braces and #ifdef's randomly. If you write code like that you
> > want to hide something and the reason I have to touch this code is
> > because there is something hiding.
> We don't take patches for 'code cleanups'. There's a reason for this.
> Customers get patches, and they don't like unnecessary lines of code
> being changed. It can make it difficult to see what the real changes are.
You should write clean code from the start. At WebKit.org we have clear Coding
Style Guidelines and most of it is enforced by review and pre-commit scripts.
Maybe adopting such things for Qtopia would make sense?
> > - I care little about BC, if I need to add a parameter to a method I do
> > so, the commit messages state so though.
> well, we do. or try to. Especially in a .x release mode.
Sure. this is why I write this into the commit message...
> > Example:
> > - So I decided to solely rely on %CPI information for our code. One way
> > would have been to just remove the timers from the libraries and be
> > happy. If I did so you could have claimed that I don't think about other
> > devices. So as I care about the consequences for other devices where
> > timers might be needed I added methods to make starting of timers
> > optional. So just because some modems are worse than the TI Calypso you
> > shouldn't punish modems that are better....
> So our customers buggy modems are just supposed to not work?
Please reread the paragraph. As I said I made starting of timers optional. So
what is the default? Default is still the old and error prone (from my point
of view and every OpenSource/FreeSoftware engineer I have talked to)
First of all I doubt that you need to start any timers to guess when something
is done. CLCC should always work (if %CPI or such vendor extensions do not
exist). So instead of guessing when a call might or might not be missed, when
all information arrived one could start your missed timer and then check the
state of the call... So I think the "buggy modem" is just an excuse for
writing fragile software.
Basing everything on timers is IMO stupid but race free. You will present
something consistent (but slightly wrong) to the user.
Basing everything on %CPI is the right thing to do (if it is provided and
reliable). You will present something consistent to the user.
Mixing both will create funny and all sort of race conditions. You get a %CPI
shortly after one of your timers fired...
So with the patches I created you are able to disable the timer guessing
completely. This fixed at least five different bugs QA reported. I left the
default to be broken to keep your "customers" happy.
Another funny race comes from the fact when a %CPI arrvies after sending a
wakeup command and QAtChat just ignores these... E.g. the approach mickeyl
has taken for FSO is to send a command that will never echo/print something
so you don't need to throw away stuff from the modem in a certain window... a
much saner approach to the same problem.
And really you should not guess. If you send ATA to a modem it might fail.
Just because you are going to queue the ATA does not mean that your call is
connected. One has to wait for the response of the modem and can do the state
transition after one knows which state the modem is in. Anything else is by
any engineering standard error prone and just asking for random failures. A
modem is a separate system, it runs software too, you should not guess about
the internal state of this unknown modem firmware. You _have_ to wait for the
modem to tell you the result of a command. So these class of issues with the
code is not about style it is about engineering practices to create reliable
> And again I fix this kind of things because it failed in reality..
> > not because I have fun thinking about all the possible race conditions
> > available in the Qtopia source.
> > - I think the approach and implementation is reasonable. If you don't
> > think so open up and start discussing it.
> we do internally. we can't discuss this publicly because it involves
> customers that do not want to be open/discussed or have their
> competitors know what they are doing/what hardware/software bugs they
> are seeing.
So discussing my patches involves customers that are not interested in sharing
improved versions with us?
> >> As well, we have been working on getting 4.4 ready to release. There are
> >> quite a few changes in 4.4, and quite a few have to be manually
> >> integrated and such.
> > well, release Qtopia4.4 beta's rsync. I can see the upcoming Qt 4.5 code
> > today as well, so what is keeping you from releasing Qtopia4.4 snapshot
> > code today?
> All I can say is that there is some discussion over this (always has
> been), and there is agreement down here over certain things.
what is the agreement?
More information about the community