[linux-dvb] Re: why emergency exit on channel switching problems ?

[Carsten Koch <Carsten.Koch@icem.de>]

Klaus Schmidinger wrote:
....
> Of course you're free to implement another workaround, but I believe
> we should invest our time in fixing the original problem...


I totally agree with Klaus on this.

This is getting ridiculous. 
The watchdog timer already is a workaround for the fact,
that the driver behaves differently, depending on the
amount of time it has run and on the number of channels 
you have switched during that time.

I see no point in stacking workarounds on top of workarounds.
Klaus is right, the original problem must be fixed.


I would love to help out in this area, but unfortunately I can't.
I believe that it will be very hard for any external developer
to contribute to the driver code, as long as it contains large
amounts of hardcoded numbers and abbreviated words.

Here is an example:

        if (i2c_writereg(dvb, 0x20, 0x00, 0x00)==1) {
                dvb->adac_type = DVB_ADAC_CRYSTAL;
                i2c_writereg(dvb, 0x20, 0x01, 0x08);
                i2c_writereg(dvb, 0x20, 0x02, 0x09);
                i2c_writereg(dvb, 0x20, 0x03, 0x20);
                i2c_writereg(dvb, 0x20, 0x04, 0x20);
        }

It would take me hours if not days to fully understand these 7 lines of code.
If all those 0x20s etc. were symbols (like the DVB_ADAC_CRYSTAL),
that time could be reduced to seconds.
Also, in many cases it may be useful to spell out more words.
For example, I associate ADAC with "Allgemeiner Deutscher Automobil Club".
I am almost certain that this is not what the ADAC in the above example means. ;-)

So, here are my top 3 constructive suggestions to the driver developers:

1.  Replace all hardcoded numbers (except, in some cases, 0 and 1)
    in the source text with symbolic names.

2.  Create a glossary of the most common abbreviations (like i2c)
    and spell out all words not listed in the glossary.

3.  Make sure all externally callable functions carefully check
    all error conditions. Return the appropriate error code on possible 
    error conditions and raise an assertion on impossible error conditions.

Believe me, I am not trying to negatively criticize your great work.
Honestly! I am trying to help.
I follow the same practise in my own code and I enforce it in code
that I have to review at work. It really helps a lot.


Carsten.


-- 
Info:
To unsubscribe send a mail to listar@linuxtv.org with "unsubscribe linux-dvb" as subject.