[Arduino] Optiboot issue

[oric_dan]

Hi Felix, I thought I would mention this in passing. I was just looking at your source code for DualOptiboot, and it's clear it uses westfw's Arduino Optiboot as a base. Just as a point of information, I wanted to call your attention to the following bug in Optiboot - as described in the very first post. The code does "work" ok for the 1284 chip, only by virtual of the 2 errors offsetting each other.
http://forum.arduino.cc/index.php?topic=241103.0

[Felix]

Yikes! Nice find sir! 
So while the bug does incidentally cause a working case for the 1284P, I think the consensus is to read the high byte as well right?

[oric_dan]

I'm not sure what the current situation is with Optiboot in Arduino-Land. I am so far only using up to IDE v.1.0.5.

As noted in post #1 of that thread, the original version of the bootloader did it correctly, but somehow it got changed in Optiboot so the high-byte was ignored. I addended a post to that thread to see if it has been fixed.

[Felix]

I will likely add the fix to DualOptiboot while that is fixed in the original Optiboot. I don't see why the high byte should be skipped since it's in the protocol spec.
As a side note .. I was wondering why the flashing of the 1284 was significantly faster. This explains why. The data page lengths are 256 whereas for the 328p they must be smaller.

[oric_dan]

Tracing this down, it appears that the "latest" version of the IDE v.1.6 still uses the 3-year old version of optiboot.c, so appears to be no attention to the issue.
https://github.com/arduino/Arduino/tree/master/hardware/arduino/avr/bootloaders/optiboot
https://github.com/arduino/Arduino/tree/master/hardware/arduino/avr
https://github.com/arduino/Arduino/
http://arduino.cc/en/Main/Software  <-- click on Source Code

As of the original thread, an exception was posted but it wasn't acted upon, and is even missing from the exception summary list, in any case. Hardly news, I guess.
https://code.google.com/p/optiboot/issues/detail?id=104
https://code.google.com/p/optiboot/issues/list

[oric_dan]

Tracking this down further, westfw fixed optiboot.c, but apparently the IDE people chose not to incorporate the fix in the latest IDE, as mentioned. Variable 'length' was changed to uint16_t, and the high byte picked up. You can see the fix here,

https://code.google.com/p/optiboot/source/detail?r=a10ef5c935a11a0e656927cd5e9b306937203f52

[Felix]

Great, I was not aware. I will look forward to upgrading DualOptiboot to this new v6.

[oric_dan]

No one will see any difference, but at least the code will correct, :-).

[Felix]

It looks like the new v6 is greater in size and I'm running out of time trying to figure out the errors, there's some overlap of the version section with the text section, and the changes are non-trivial to adapt to DualOptiboot. So i'd rather not mess with it if it works the way it is right now.

[oric_dan]

I notice they kind of did the usual Arduino whiz-bang-obfuscation, cover-all-cpus-etc, dog-and-pony thing.

Things work ok as is, but you could probably fix your bootloader by simply changing 'length' from uint8_t to uint16_t, and then catching the upper byte to length. Only takes adding one extra line of code, but can probably just not worry about it, in any case. At least you know about the issue, :-).

I mean, Arduino-Centrale chose not to include the fix in the IDE, and optiboot is being burned into 1000s of chips.

[Felix]

You mean in millions
Yeah i thought the change to uint16_t would do it but i tried to follow their steps and there's a lot more to it than that, they completely changed the way they read and process the data pages so i gave up at that point. I'll have to revisit it and see if I can figure it out in my own now-"old" source.

[oric_dan]

I meant to say 1000s of chips "every day", LOL.

But I think all you need to do is change to this:

Code Select Expand

uint16_t length;

      length = getch() << 8;			/* getlen() */
      length |= getch();
      getch();

And do the same thing in the STK_READ_PAGE section. I don't think the desttype [3rd getch()]matters here. It's ignored in the old optiboot, and only used to distinquish EEPROM from Flash during read/writes in the new one.

In any case, whatever seems best. I'm sure you don't need to worry too much about it, if A-Centrale isn't.

[TomWS]

...
I mean, Arduino-Centrale chose not to include the fix in the IDE, and optiboot is being burned into 1000s of chips.

This is probably WHY they aren't changing the code.  It's like a HUGE Tanker ship sailing on the seas and this would be trying to change direction.  It takes a lot of effort to make that change. There had better be a VERY good reason to change. 

I've been in that place where it was just a 'little' change but to a huge volume product - uh, the gods have to intervene in that case.

Of course, Felix is in the same place, HUGE volumes and so little time...   

Tom