NUM_SERVOS > 1 compile time error

[hex4def6]

Compile Error

When I compile with NUM_SERVOS > 1 I get an error:

sketch/servo.cpp: In member function 'void Servo::move(int)':
servo.cpp:312: error: static assertion failed: SERVO_DELAY must be an array NUM_SERVOS long.
   static_assert(COUNT(servo_delay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long.");
   ^
exit status 1
static assertion failed: SERVO_DELAY must be an array NUM_SERVOS long.

I believe commit e9c7297 is to blame; It added the following:

  constexpr uint16_t servo_delay[] = SERVO_DELAY;
  static_assert(COUNT(servo_delay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long.");

My C is rusty, but my understanding is that what is being asserted is not what was done to the array; shouldn't the array length be declared as NUM_SERVOS long if you're going to test against that?

I'm actually confused what this commit is trying to achieve; is the idea that each servo could have a separate delay (hence the array)? If so, there doesn't appear to be a way of setting them individually.

[AnHardt]

Extend the array until big enough for NUM_SERVOS

#define SERVO_DELAY { 300, 300 }

[hex4def6]

Ah, of course. Thanks.

[hex4def6]

I'd recommend placing something in the comment text & and/or assert message about that, something like "SERVO_DELAY should be comma separated list of values equal to NUM_SERVOS. For example, for a three servo configuration, you could use SERVO_DELAY { 300, 300, 300 }"

[Roxy-3D]

In C, if you don't specify the full number of initialization numbers... It should just replicate through the array.

int a[4] = {300};

should just fill the entire thing with 300's ????

[hex4def6]

Well, with this in configuration.h:

#define NUM_SERVOS 2 // Servo index starts with 0 for M280 command

// Delay (in milliseconds) before the next move will start, to give the servo time to reach its target angle.
// 300ms is a good value but you can try less delay.
// If the servo can't reach the requested position, increase it.
#define SERVO_DELAY { 300 }

I get the issue described above. If I set servo_delay to:

#define SERVO_DELAY { 300, 300 }

It works.

Likewise, if I set NUM_SERVOS to 1, but leave SERVO_DELAY with { 300, 300} the assert fails (as it should).

So, I think it's working as advertised, it's just not very clear from the configuration.h comments that SERVO_DELAY is now an array.

[DerAndere1]

It should indeed be explained in the comments of configuration.h that SERVO_DELAY is a braced init list for an array. Since C++ now has uniform initialization, that fact is not obvious from the syntax.
@Roxy-3D : I had the same missunderstanding as @hex4def6 so I would not call this issue as resolved in marlin 2.0 bugfix.

[DerAndere1]

Alternatively, change line 150 in Marlin/src/HAL/shared/servo.cpp from:
constexpr uint16_t servo_delay[] = SERVO_DELAY;
to the following:
constexpr uint16_t servo_delay[NUM_SERVOS] = SERVO_DELAY;

Then, in configurations.h, SERVO_DELAY could be either
a) a braced init list with 1 element like {300} OR (at the user's discretion)
b) a braced list of n initializers (where n = NUM_SERVOS), e.g. {300, 300}

[DerAndere1]

See also closed issues #7254, #7528, #8351

[thinkyhead]

Last I checked, elements that are left out in an initializer list get initialized to 0, not to the single provided value. So this:

constexpr uint16_t servo_delay[3] = { 300 };

…actually gives you this…

constexpr uint16_t servo_delay[3] = { 300, 0, 0 };

[DerAndere1]

Oops. Anyway, once the documentation (e.g. at http://marlinfw.org/docs/configuration/configuration.html#extras-2 ) is updated after Marlin 2.0 is stable, this won't be an issue.

PS: Thanks for correcting me. I used too much Python recently. Marlin is great, I use it for my DIY lab robot (PipetBot-A8).

[thinkyhead]

Oy, yeah… The configuration documentation needs a major revision, and needs to be split up into several pages (most likely based on the @section directives in the files). It would also make sense to have a 1.1.9 version and a 2.0.x version because they have diverged a lot.

[Sidans]

I am new to these stuff .
I have replaced the inductive auto leveling sensor with a BLTOUCH and following an instruction from you tube to configure BLTOUCH. the instruction says the number for NU_Servo should be changed to 1 from 2 ( my current setting )
can some one please explain to me what do we mean by NUM_Servo here. is it the number of servo motors on the printer? if yes, most printers have at least 3 motors. why 1 instead of 3?
thanks

[DerAndere1]

@Sidans regarding #7734 (comment).
Welcome to the Marlin repository at github. It's good you looked for related issues before creating a new one. However you have a question regarding usage of Marlin and you obviously have not been observing the error discussed here. In future, try the following first, before using the issue tracker at github:

1. read the fine manual (like for any other software/hardware)
2. If you do not want to discuss a feature request or bug report:

Do you want to ask a question? Are you looking for support? Please don't post here. Instead please use the Marlin Firmware forum at http://forums.reprap.org/list.php?415 or the Marlin Facebook Group https://www.facebook.com/groups/1049718498464482/.

3. You can simply look for a [more related issue]: How do I set up BLTouch? #5142.

After reading the very specific instructions for configurating Marlin1.1.x for BL Touch and the instructions for BL Touch found using google, you might realize that BL Touch is controlled in a similar way to an RC servo. (not to be confused with stepper motors).

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.