#ifdef by properties, not by chip names: improve support for uncommon chips.

[Traumflug]

Two years ago I started Generation 7 Electronics, which is pretty close to an Arduino from the programmers view. Back then there was no support for 20 MHz chips, ATmega644 (without P) and ATmega644P, so I had to tweak the library fork made for the Sanguino. Today, things have changed, thanks a lot. I just rebased my derivate to Arduino 1.0.3 and I think the patch has become small enough to deserve incorporation into the general Arduino library.

My strategy is to check not for chip names, but for their features. This way you become compatible to virtually all ATmegas and ATtinys existing, even the rare ones, and without long chains of #if defined().

For the result, please see this patch against the 1.0.3 tag:

From faeb76ed6443dd0422a98e003d824fc5a7b835ab Mon Sep 17 00:00:00 2001
From: Markus Hitter <mah@jump-ing.de>
Date: Wed, 30 Jan 2013 21:01:07 +0100
Subject: [PATCH] All changes for ATmega644, 644P and 1284P in one commit.

---
 hardware/arduino/cores/arduino/Arduino.h        |   11 +++++++++--
 hardware/arduino/cores/arduino/Tone.cpp         |    5 +++--
 hardware/arduino/cores/arduino/wiring.c         |    2 +-
 hardware/arduino/cores/arduino/wiring_analog.c  |    4 ++--
 hardware/arduino/cores/arduino/wiring_private.h |    4 ++--
 5 files changed, 17 insertions(+), 9 deletions(-)
 mode change 100755 => 100644 hardware/arduino/cores/arduino/Print.cpp
 mode change 100755 => 100644 hardware/arduino/cores/arduino/Print.h
 mode change 100755 => 100644 hardware/arduino/cores/arduino/Tone.cpp
 mode change 100755 => 100644 hardware/arduino/cores/arduino/wiring_private.h
 mode change 100755 => 100644 hardware/arduino/cores/arduino/wiring_pulse.c
 mode change 100755 => 100644 hardware/arduino/cores/arduino/wiring_shift.c

diff --git a/hardware/arduino/cores/arduino/Arduino.h b/hardware/arduino/cores/arduino/Arduino.h
index b265825..349b9a4 100755
--- a/hardware/arduino/cores/arduino/Arduino.h
+++ b/hardware/arduino/cores/arduino/Arduino.h
@@ -41,12 +41,16 @@ extern "C"{
 #define FALLING 2
 #define RISING 3

-#if defined(__AVR_ATtiny24__) || defined(__AVR_ATtiny44__) || defined(__AVR_ATtiny84__) || defined(__AVR_ATtiny25__) || defined(__AVR_ATtiny45__) || defined(__AVR_ATtiny85__)
+// analog voltage references
+#if defined(REFS2) // ATtiny of some sort
 #define DEFAULT 0
 #define EXTERNAL 1
 #define INTERNAL 2
 #else  
-#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__) || defined(__AVR_ATmega1284P__) || defined(__AVR_ATmega644P__)
+// Looks like the SFIOR register exists only on those chips without
+// 1.1V internal reference voltage. This is an educated guess inspired
+// by AppNote AVR097, page 5 bottom.
+#if !defined(SFIOR)
 #define INTERNAL1V1 2
 #define INTERNAL2V56 3
 #else
@@ -65,7 +69,10 @@ extern "C"{
 #define max(a,b) ((a)>(b)?(a):(b))
 #define abs(x) ((x)>0?(x):-(x))
 #define constrain(amt,low,high) ((amt)<(low)?(low):((amt)>(high)?(high):(amt)))
+#if __GNUC__ <= 4 && __GNUC_MINOR__ < 5
+// this conflicts with avr-gcc's built in headers as of avr-gcc 4.5.3
 #define round(x)     ((x)>=0?(long)((x)+0.5):(long)((x)-0.5))
+#endif
 #define radians(deg) ((deg)*DEG_TO_RAD)
 #define degrees(rad) ((rad)*RAD_TO_DEG)
 #define sq(x) ((x)*(x))
diff --git a/hardware/arduino/cores/arduino/Tone.cpp b/hardware/arduino/cores/arduino/Tone.cpp
index 9bb6fe7..964f2ec
--- a/hardware/arduino/cores/arduino/Tone.cpp
+++ b/hardware/arduino/cores/arduino/Tone.cpp
@@ -37,7 +37,7 @@ Version Modified By Date     Comments
 #include "Arduino.h"
 #include "pins_arduino.h"

-#if defined(__AVR_ATmega8__) || defined(__AVR_ATmega128__)
+#if defined(TCCR2) // ATmega8, ATmega128 or similar
 #define TCCR2A TCCR2
 #define TCCR2B TCCR2
 #define COM2A1 COM21
@@ -54,7 +54,7 @@ Version Modified By Date     Comments
 //  = 0 - stopped
 //  < 0 - infinitely (until stop() method called, or new play() called)

-#if !defined(__AVR_ATmega8__)
+#if defined(TIMSK0)
 volatile long timer0_toggle_count;
 volatile uint8_t *timer0_pin_port;
 volatile uint8_t timer0_pin_mask;
@@ -86,6 +86,7 @@ volatile uint8_t timer5_pin_mask;
 #endif

+// Don't distinguish by chip type, but by register presence
 #if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)

 #define AVAILABLE_TONE_PINS 1
diff --git a/hardware/arduino/cores/arduino/wiring.c b/hardware/arduino/cores/arduino/wiring.c
index ac8bb6f..8407849 100644
--- a/hardware/arduino/cores/arduino/wiring.c
+++ b/hardware/arduino/cores/arduino/wiring.c
@@ -41,7 +41,7 @@ volatile unsigned long timer0_overflow_count = 0;
 volatile unsigned long timer0_millis = 0;
 static unsigned char timer0_fract = 0;

-#if defined(__AVR_ATtiny24__) || defined(__AVR_ATtiny44__) || defined(__AVR_ATtiny84__)
+#if defined (TIM0_OVF_vect)
 SIGNAL(TIM0_OVF_vect)
 #else
 SIGNAL(TIMER0_OVF_vect)
diff --git a/hardware/arduino/cores/arduino/wiring_analog.c b/hardware/arduino/cores/arduino/wiring_analog.c
index 23b01c6..38c8e39 100644
--- a/hardware/arduino/cores/arduino/wiring_analog.c
+++ b/hardware/arduino/cores/arduino/wiring_analog.c
@@ -41,11 +41,11 @@ int analogRead(uint8_t pin)
 {
    uint8_t low, high;

-#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
+#if defined(PORTL) // 100 pin chips
    if (pin >= 54) pin -= 54; // allow for channel or pin numbers
 #elif defined(__AVR_ATmega32U4__)
    if (pin >= 18) pin -= 18; // allow for channel or pin numbers
-#elif defined(__AVR_ATmega1284P__) || defined(__AVR_ATmega644P__)
+#elif defined(PORTA) // 40/44 pin chips
    if (pin >= 24) pin -= 24; // allow for channel or pin numbers
 #else
    if (pin >= 14) pin -= 14; // allow for channel or pin numbers
diff --git a/hardware/arduino/cores/arduino/wiring_private.h b/hardware/arduino/cores/arduino/wiring_private.h
index f678265..c91289e
--- a/hardware/arduino/cores/arduino/wiring_private.h
+++ b/hardware/arduino/cores/arduino/wiring_private.h
@@ -52,9 +52,9 @@ extern "C"{
 #define EXTERNAL_INT_6 6
 #define EXTERNAL_INT_7 7

-#if defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
+#if defined (SIG_INTERRUPT7)
 #define EXTERNAL_NUM_INTERRUPTS 8
-#elif defined(__AVR_ATmega1284P__) || defined(__AVR_ATmega644P__)
+#elif defined (SIG_INTERRUPT2)
 #define EXTERNAL_NUM_INTERRUPTS 3
 #elif defined(__AVR_ATmega32U4__)
 #define EXTERNAL_NUM_INTERRUPTS 4
-- 
1.7.10.4

What do you think? Given this could go into this repo I could give up my derivate, making things for many users much easier. Not only Gen7 users, but any user of a rare AVR.

[damellis]

I can't speak for the details but this is definitely the kind of thing that would be good to include. Having the core be as generic as possible is useful.

Traumflug, can you comment on how you've tested these changes? It can be easy to break things on some board in making this kind of modification, so the more boards you can try out, the more likely it will be that the patch will be accepted.

[Lauszus]

I think it would be really nice if this got merged. Traumflug I will be happy to test it for you. I got several Arduinos I can try it on and some other atmega chips as well.

[lestofante]

I like this too Il giorno 01/feb/2013 15:15, "Kristian Lauszus" notifications@github.com ha scritto:

I think it would be really nice if this got merged. Traumflug I will be happy to test it for you as well if you as well. I got several Arduinos I can try it on.

— Reply to this email directly or view it on GitHubhttps://github.com/arduino/Arduino/issues/1258#issuecomment-12995702.

[Traumflug]

Traumflug, can you comment on how you've tested these changes?

These patches were made by verification with the avr-libc headers. Looking into them one can see there is a comparably small number of base architectures plus a few chip-specific modifications. Atmel was nice enough to give the same register names to all chips with the same feature, so you simply check for the presence of a register definition to find out wether a feature exists.

Regarding testing: for obvious reasons I couldn't do that by try and error on a dozen ATmegas with a hundred pieces of software :-) They run for about a year so far on those RepRap controllers which are not based on the ATmega2560. Only two or three different softwares but a fairly large number of users.

[Traumflug]

P.S.: Testing is simple: you can download and install this package: https://github.com/Traumflug/Generation_7_Electronics/blob/Gen7-Arduino-IDE-Support-2.1/release%20documents/Gen7%20Arduino%20IDE%20Support%202.1.zip?raw=true Installation instructions are inside the package.

To use the Gen7 library, which differs from the standard one only by the above patch, adjust this line in your boards.txt file (inside the Arduino IDE folder):

<your board>.build.variant=gen7

[damellis]

Sorry, I should have been more specific. I'm not asking you to test hundreds of programs on dozens of Arduino boards. But if you've checked that it doesn't break anything obvious on the processors on the official Arduino boards (ATmega328, ATmega2560, and ATmega32U4), that would be good to know. We've had patches like this in that past that, in trying to get things to work on e.g. the ATmega644, have broken analogRead() completely on, say, the Arduino Mega. We'd obviously like to avoid something like that.

[Traumflug]

Obviously. I must admit, I own a whole bunch of Arduino-like RepRap controllers, but not a single original Arduino.

In case there are known failed attempts I can help debugging, of course. By reading avr-libc headers even more closely.

[Traumflug]

@Lauszus: did you have a chance to test the patch on some of your Arduinos?

@damellis: Is there a list of the ATmega types Arduino cares about? If I could get such a list I could write sort of a unit test to make sure the same code is compiled with or without the patch.

[Lauszus]

No I havn't tested it yet. I can test it on an Uno, Mega and Leonardo. Just tell me what you want me to test and I will have a look at it!

[damellis]

The most important AVR's are the ATmega328P, ATmega2560, and ATmega32U4. It would also be good to check the ATmega168 and ATmega8, as they're on older boards.

[Lauszus]

I can confirm that all the interrupts are working on ATmega328P, ATmega2560, and ATmega32U4.

[Lauszus]

But INTERNAL1V1 and INTERNAL2V56 is getting defined when you are using an ATmega328P or ATmega32U4, so that has to be fixed.