Re: [avrdude-dev] Butterfly (was: Before a release...)

[Jan-Hinnerk Reichert]

On Thursday 27 November 2003 18:04, Michael Mayer wrote:
> On Thu, Nov 20, 2003 at 19:37:08 +0100, Jan-Hinnerk Reichert wrote:
> [reporting the butterfly patch works for his AVR910]

> I agree with you, even home-patched AVR910 versions have to work
> with the patched driver. But I wonder what is the best way to do
> this.

From the user perspective it doesn't matter how you do it. As long as 
it works, of course.

> You suggest the definition of a subtype field, Eric prefers to
> define a new type. Today I checked out the impact of both
> alternatives to the overall structure of the sources. The
> subtype-way needs only very small modifications in pgm.h,
> config_gram.y and avr910.c leaving all the structure intact, with
> sounds good to me.

Actually, I have changed my mind and agree with Eric now (I just 
realized that I had accidently replied to Eric only ;-(

> Introducing a new type is more difficult, because it would need
> something like butterfly.c which duplicates most of the
> functionality of avr910.c making it difficult to keep the common
> parts in sync. One possible solution may be to split avr910.c in
> two parts, one specific part and a part, which is shareable with
> the new, small butterfly.c. But this will break the current
> modularisation scheme, some of the static functions in avr910 will
> have to be public. In detail all these functions should be shared
> between avr910.c and butterfly.c and should be moved to a new file
> like avr_common.c oder serial_common.c:

I think that duplicating the code is the easier way. All these 
conditional statements make it hard to understand and maintain the 
code.

The butterfly would probably become even shorter and clearer than the 
avr910 (e.g. you can just put an error if a write byte for flash 
occurs)

[List of functions]

Most of the duplicated functions are empty anyway. Perhaps we should 
change the default-functions in "pgm.c". So, that non-critical 
functions - like setting a LED - don't return an error, if they are 
undefined

> In order to avoid the split, one can simply add the butterfly
> specific parts to avr910, so one file defines two different
> programmer types. But again, the clean structure gets lost.

No way ;-)

> I would like to avoid messing up the structure and propose to do a
> autodetection to distinguish between a real avr910 and a butterfly.
> Currently, I use the response to the new 'b'-command. Not the best
> choice, as Jan mentioned.

I still feel uneasy autodetection.

> The id string returned from the 'S'-command can be used as well.
> The S-command is well defined and a valid answer is required
> anyway. The butterfly returns "AVRBOOT", is this string unique or
> is there a avr910 which does return this, too?

The avr910-appnote says that everything that begins with "AVR" is an 
avr910 prommer. IIRC, the bootloader-appnote from Atmel also uses 
"AVRBOOT".

> Or the 'v'-command: A real avr910 is expected to return a two byte
> string showing the hardware version. The butterfly doesn't
> implement this command and just returns a single '?'.

This could change in a future version of the Butterfly-firmware. So 
the "b" still seems the best way, if one wants autodetection.

> What do you think about this proposal?

I think the additional 15k from duplicating the code is worth it, 
since it increases readability/maintainability of the code. The user 
has to specify a programmer and a chip anyway, so he won't care.

IMHO, future changes in the code are easier, if programmers are 
completely separated. Also, testing changes is easier, if they only 
affect one programmer (especially, if you have no access to the other 
one ;)

/Jan-Hinnerk