[avrdude-dev] Re: avrdude 4.2.0 Release

[Theodore A. Roth]

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?

Sounds like you have the jist of it.

:)
:)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 :-)

I think that is what will need to happen. I'll verify the patch as is and 
commit along with a note in the TODO file.

:)
:)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.

That sounds possible, but may not help maintainability.

I'm thinking (out loud here) that avr.c might need a bit more work to make
things cleaner in the long run. What about this:

  - Everything in avr.c is page based. The lower level stuff can then take a 
    page and send it a byte at a time if needed. The current logic is
    starting to get a bit crazy.

  - Do all the demuxing of memspace in avr.c. Currently, all the backends 
    have to duplicate this. Then we'd have methods in each backend for each
    memory space.

Of course, this is post-release stuff.

:)
:)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 :-)

Actually is useful. We're both thinking the same way... I don't like side 
effects

Ted Roth