Pinoccio / firmware-pinoccio

Collects the main pinoccio sketch and all libraries it depends upon
20 stars 13 forks source link

Does not compile with Arduino 1.0.x #10

Closed matthijskooijman closed 9 years ago

matthijskooijman commented 10 years ago

This is somewhat intentional - we use features from the 1.5.x IDE, most notably the updated toolchain. However, @sej7278 has been trying to get pinoccio to compile using the 1.0.x core, together with Arduino-Makefile (which allows customizing build flags to e.g. add C++11 support) and the avr-gcc toolchain included with Debian (which is new enough to support our chip). Previous discussion was here: https://github.com/sudar/Arduino-Makefile/issues/243#issuecomment-52828981 but I didn't want to further litter that repository, so this is a better place.

It seems unlikely that we'll support the actual 1.0.x IDE, but perhaps we can support the 1.0.x Arduino core in the above scenario with some non-invasive changes, which is what we'll be investigating here.

@matthijskooijman i actually got it to build with the 1.0-style boards.txt and Makefile above, if i deleted HardwareSerial.cpp

Does the pinoccio not have hardware serial, but because its using the arduino core, it tries to pull in HardwareSerial.cpp despite line 33 ?

Pinoccio does use HardwareSerial, it even has 2 UARTS. If you deleted HardwareSerial.cpp, then you'll probably have broken Serial support in the process...

I'm also not sure why you'd want to delete HardwareSerial.cpp. At some you mentioned that pins_arduino.h lacked some HardwareSerial-related defines, could you elaborate on that? Sounds like some of this stuff was changed between 1.0 and 1.5 and we might be able to add some defines to support both?

sej7278 commented 10 years ago

boards.txt i made in 1.0 format, essentially making two board types by duplicating the menu.cpu entries:

##############################################################

pinoccio256.name=Pinoccio Scout
pinoccio256.upload.protocol=wiring
pinoccio256.upload.maximum_size=253952
pinoccio256.upload.speed=115200
pinoccio256.build.f_cpu=16000000L
pinoccio256.build.core=arduino:arduino
pinoccio256.build.variant=pinoccio
pinoccio256.build.mcu=atmega256rfr2
pinoccio256.bootloader.low_fuses=0xDE
pinoccio256.bootloader.high_fuses=0xD0
pinoccio256.bootloader.extended_fuses=0xFE
pinoccio256.bootloader.unlock_bits=0x3F
pinoccio256.bootloader.lock_bits=0x2F
pinoccio256.bootloader.path=STK500RFR2/release_0.51
pinoccio256.bootloader.file=boot_pinoccio.hex

##############################################################

pinoccio128.name=ATmega128RFA1 (early prototype)
pinoccio128.upload.protocol=wiring
pinoccio128.build.f_cpu=16000000L
pinoccio128.build.core=arduino:arduino
pinoccio128.build.variant=pinoccio
pinoccio128.build.mcu=atmega128rfa1
pinoccio128.upload.maximum_size=129048
pinoccio128.upload.speed=57600
pinoccio128.bootloader.low_fuses=0xDE
pinoccio128.bootloader.high_fuses=0xD0
pinoccio128.bootloader.extended_fuses=0xFE
pinoccio128.bootloader.unlock_bits=0x3F
pinoccio128.bootloader.lock_bits=0x2F
pinoccio256.bootloader.path=STK500RFR2/release_0.51
pinoccio256.bootloader.file=boot_pinoccio.hex

##############################################################
sej7278 commented 10 years ago

i renamed the avr directory from core-pinoccio to "pinoccio" so i ended up with:

~/sketchbook/hardware/pinoccio/
   bootloaders/
   variants/
   boards.txt

i then made this Makefile, taking some of the settings from platform.txt:

BOARD_TAG         = pinoccio256
ALTERNATE_CORE    = pinoccio
BOOTLOADER_PARENT = ~/sketchbook/hardware/pinoccio/bootloaders
BOOTLOADER_PATH   = STK500RFR2/release_0.51
BOOTLOADER_FILE   = boot_pinoccio.hex
CFLAGS_STD        = -std=gnu99
CXXFLAGS_STD      = -std=gnu++11

include /usr/share/arduino/Arduino.mk
sej7278 commented 10 years ago

Log output of the build attempt gist using Arduino-Makefile as of commit 47c9ec8f6

sej7278 commented 10 years ago

I only deleted HardwareSerial.cpp as that seemed to be the cause of all of the problems. Sure it would have disabled serial support, but it did allow the build to finish.

matthijskooijman commented 10 years ago

Hmm, it seems that this part of HardwareSerial.cpp is causing the problem: https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/HardwareSerial.cpp#L263-L278

It looks like USART2_UDRE_vect is defined, but not UCSR2B and other registers. Since the pinoccio only has 2 UARTS (0 and 1), it's weird that USART2_UDRE_vect would be defined. I think this is actually an error in the avr-libc include file (/usr/lib/avr/include/avr/iom256rfr2.h).

In an older version of the 1.5.x HardwareSerial.cpp, it seems there was already a workaround for this: https://github.com/arduino/Arduino/blob/0bd6a2d20fb9664255b20e0db11dd4586ebe9007/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp#L120d (note that since then, the HardwareSerial implementation in 1.5.x was significantly changed in a way that also, accidentally, works around this problem).

Your best bet is probably modify your HardwareSerial.cpp to include that additional check - I can't think of any way to fix this using e.g. compiler options.

So, this is really an avr-libc bug. I'd report it upstream, but the version in avr-libc trunk does not seem to have this problem. It never had it in the first place, it seems that the iom256rfr2.h shipped with Debian and Arduino are taken from Atmel's 3.4.3 and 3.4.4 toolchains (http://distribute.atmel.no/tools/opensource/Atmel-AVR-GNU-Toolchain/3.4.3/avr8-headers-6.2.0.142.zip).

sej7278 commented 10 years ago

wow, good analysis!

it would seem that debian changed to using the atmel toolchain as avr-libc development ceased around 2010 ( bug / changelog ) and even then most of the work was from atmel.

so are you saying that the bug is in atmel's version of iom256rfr2.h and not (non-gnu) avr-libc's?

i've no idea how to report bugs to atmel, not worth doing it at debian level as they're just importing.

edit: my HardwareSerial.cpp is identical to the L120 one you've linked to, and has the portions from the L263-278 one:


#if defined(USART2_RX_vect) && defined(UDR2)
  void serialEvent2() __attribute__((weak));
  void serialEvent2() {}
  #define serialEvent2_implemented
  ISR(USART2_RX_vect)
  {
    if (bit_is_clear(UCSR2A, UPE2)) {
      unsigned char c = UDR2;
      store_char(c, &rx_buffer2);
    } else {
      unsigned char c = UDR2;
    };
  }
#endif

....blah

#ifdef USART2_UDRE_vect
ISR(USART2_UDRE_vect)
{
  if (tx_buffer2.head == tx_buffer2.tail) {
    // Buffer empty, so disable interrupts
    cbi(UCSR2B, UDRIE2);
  }
  else {
    // There is more data in the output buffer. Send the next byte
    unsigned char c = tx_buffer2.buffer[tx_buffer2.tail];
    tx_buffer2.tail = (tx_buffer2.tail + 1) % SERIAL_BUFFER_SIZE;

    UDR2 = c;
  }
}
#endif

source: http://anonscm.debian.org/cgit/collab-maint/arduino.git/tree/hardware/arduino/cores/arduino/HardwareSerial.cpp

matthijskooijman commented 10 years ago

Hmm, it seems there is still some development upstream, e.g. last commit is from 5 days ago: http://svn.savannah.nongnu.org/viewvc/trunk/?root=avr-libc It seems the correct version of iom256rfr2.h has been in upstream trunk for over a year now: http://svn.savannah.nongnu.org/viewvc/trunk/avr-libc/include/avr/iom256rfr2.h?root=avr-libc&view=log I was going to say it isn't in any released version yet, but it seems that an 1.8.1 version containing it was released last week: http://svn.savannah.nongnu.org/viewvc/avr-libc-1_8_1-release/include/avr/iom256rfr2.h?root=avr-libc&view=log

Still, I guess atmel should really try to get their patches cleaned up and committed upstream (not sure if they've tried though).

In any case, this would mean that if atmel bumps their avr-libc to 1.8.1, they will likely drop their iom256rfr2.h file and use the correct version from upstream. Might be a while until that happens and Debian updates, so that won't really do you any good right now...

Reporting to Debian and supplying a patch to fix it in Debian might be a faster way (diffing the atmel version against the upstream version might be a good start).

sej7278 commented 10 years ago

copying svn://svn.sv.gnu.org/avr-libc/avr-libc-1_8_1-release/include/avr/iom256rfr2.h over the one included with debian makes it compile just fine.

the difference is massive - the debian/atmel file is 55k, the avr-libc one is 201k

ghost commented 10 years ago

Any progress on this? I would like to ditch the IDE asap for ease of deployment with CPP flags

sej7278 commented 10 years ago

yeah, copy that file and it works.

atmel won't be moving to avr-libc 1.8.1 in their next build apparently, which probably means debian won't be anytime soon either. so copy this iom256rfr2.h to /usr/lib/avr/include/avr (optionally backing up the old one) then use the boards.txt, Makefile and directory structure i made above.

i'd appreciate if you could test it works on the hardware as i don't have one, but it certainly compiles.

sej7278 commented 10 years ago

reported iom256rfr2.h issue upstream as https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=762303

amcjen commented 10 years ago

Super, thanks for posting this to upstream.

sej7278 commented 9 years ago

yeah they don't seem keen on fixing it in debian, they're waiting for atmel to do it for them, so fine to close it.

sej7278 commented 7 years ago

Debian finally fixed it (well just took a new atmel package) https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=762303 and we no longer need to set CFLAGS_STD or CXX-FLAGS_STD due to arduino-mk updates