Re: [avrdude-dev] Re: avrdude 4.2.0 Release

[Theodore A. Roth]

On Thu, 4 Sep 2003, Theodore A. Roth wrote:

> On Thu, 4 Sep 2003, Brian Dean wrote:
>
> :)On Thu, Sep 04, 2003 at 07:33:26PM -0700, Theodore A. Roth wrote:
> :)
> :)> I think this patch breaks the encapsulation of the avr910 module. It
> :)> makes calls to avr_write_byte_default() and
> :)> avr_read_byte_default(). I'm not sure if that is the best route to
> :)> take since if may come back to bite us in the future if there are
> :)> changes made to avr.c.
> :)>
> :)> Brian, what's the policy here, if any?
> :)
> :)Let me see if I understand this - the avr910 currently doesn't support
> :)writing fuse bits, nor did it used to support a "universal command"
> :)command?  And now, the new AVR910 firmware supports a "universal
> :)command" which can be used to access fuse bits?  So the idea is that
> :)the prefered byte-writing function for flash and eeprom is
> :)avr910_write_byte(), but if we happen to be writing something other
> :)than flash or eeprom (presumable fuse bits), then we instead call the
> :)"default" byte writing routine which happens to call the "universal
> :)command" for the programmer if it has one?  Is that close?
> :)
> :)I tend to agree that several assumptions are being made about what
> :)avr_write_byte_default() does and how it works.  Can you think of a
> :)cleaner way to do the equivalent without too much code duplication and
> :)without too many changes just before the release?
> :)
> :)I'm OK with whatever you decide.  It's also fine with me if you decide
> :)to let this go in as is and then maybe revisit the issue after the
> :)release to make it work cleaner, in all your free time, of course :-)
> :)
> :)Maybe something as simple as renaming 'avr_write_byte_default()' to be
> :)something like 'avr_write_byte_universal()', and create a new
> :)'avr_write_byte_default()' to just call that.  Then, in
> :)avr910_write_byte() make it call the "universal" command directly.
> :)That way we are still free to change the implementation of
> :)avr_write_byte_default() without affecting anything else.  Just a
> :)thought.
> :)
> :)In direct answer to your question, there is no strict policy, but my
> :)hope is that the code stays (becomes) clean(er) and doesn't devolve
> :)into a maintenance nightmare where little changes here have unexpected
> :)results over there.  I know, not a very useful answer :-)
>
> I've tested the attached patch as best I can since my 8515's don't support
> SPI access to fuses and my only mega163 is temporarily hosed with the SPIEN
> fuse disabled (need HiV to fix it). :-\ Seems to me it issues the universal
> commands correctly.
>
> It's a little bit of a tweak to Jan's patch. His patch generated warnings
> due to avr_{read,write}_byte_default() not having protos in avr.h in
> addition to the broken encapsulation issue.
>
> Essentially the same code gets executed in both patches, but I think mine is
> a bit more clear as to what is getting called when looking at avr.c (of
> course, that's open to debate ;-).
>
> If there's no objections to my hack, I'll commit it tomorrow.

Ok, it's been committed.

Ted Roth