Arduino IDE 1.5.4 Servo library defenition refers to implementation specific file

[jantje]

The file servo.h includes ServoTimers.h which is in the arch folder.
This has 2 consequences

1. All arch implementation need a ServoTimers.h file
2. arch/[infrastructure] needs to be added to the include path

This should be fixed because a definition should never enforce an implementation.
Adding the header to the Servo.cpp file in the avr and sam fixes this problem

include "Servo.h"

// Architecture specific include

include "ServoTimers.h"

[cmaglie]

Hi jantje,

In this case you may be right, the include of ServoTimers.h can be moved inside arch without much problems (and I'll move it), but I'm not sure that this could solve the bigger problem, because removing arch/xxx from the include path will not allow you to use inlining:

http://stackoverflow.com/a/9317494
http://www.parashift.com/c++-faq/inline-member-fns.html

(I'm coming back also to the forum thread, thanks for your patience)

C

[jantje]

C
This issue has nothing to do with the problems I have to implement library specification 1.5 in Eclipse.
It has to do with a breach against separating definition and implementation.
servo.h is clearly a definition where ServoTimers.h is a hardware specific implementation (because there is a different one for sam and avr).
By including ServoTimers.h in servo.h Servo.h is no longer a architecture independent definition file. The library specification 1.5 is unclear whether this is ok or not; for me it definitely goes against a layered approach.

IMHO to solve inline (and other) issues it makes more sense that an architecture specific Servo.h file includes ../../header/Servo.h which is the other way around of what we have now. (note I say header and not src)
I can see that as "a way to overcome the drawbacks of overloading a pure virtual Servo class". It does not allow for extensions but it does allow for inline. That is the same functionality with the current solution.

Best regards
Jantje

PS If it is about inline I would have picked a different name then ServoTimers.h.

[cmaglie]

As I said, you're right about the Servo library, I've just moved the include in right place.
C

[cmaglie]

About the "layered" approach, I like it, but the the arch/[platform] is in the include path because things are not so straightforward in C++.

In particular, in-lining a function called from the sketch, is not possible without also including the whole body of the function (this is is the first thing that popped up in my mind, but may be some other)

C

[cmaglie]

I reverted this one because StandardFirmata didn't compile anymore: it uses the constant MAX_SERVO to statically allocate arrays to manipulate servos.

It's defined like that:

#define MAX_SERVOS (_Nbr_16timers * SERVOS_PER_TIMER)

where _Nbr_16timers is hardware dependent.

C