[BUG] Servo library broken on skr pro

[MangaValk]

Bug Description

My Configurations

put the following lines in the skr pro pins file

#define SERVO0_PIN         PA1 // bltouch
#define SERVO1_PIN         PF8

set #define NUM_SERVO 2 in configuration.h
and enable bltouch

Steps to Reproduce

run M280 on the different channels

Expected behavior: [What you expect to happen]
different servo channels all work

Actual behavior: [What actually happens]
only first channel is working.
m280 on other channels makes first channel move.

[sjasonsmith]

@MangaValk you mentioned on discord that you were looking at the code. Are you intending to work on a fix?

[MangaValk]

@sjasonsmith correct. I am trying to make a fix.
But honestly I'm a bit overwhelmed by the complexity of the HAL.
If I manage to find a fix I'll send a pull request.

[sjasonsmith]

Can you attach your configuration files?

Are you looking at the code in the the STM32 framework?
As far as I can tell, it looks like the code supports multiple servos, but they will round-robin and only pulse one of them in each 20ms period.

Keep in mind that this is from me reading code only, and not actually testing anything.

I assume you already had to resolve this failing static assert before you were able to compile at all?
static_assert(COUNT(servo_delay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long.");

[MangaValk]

I was looking in the ./Marlin\src\HAL\HAL_STM32 folder at the servo files. i'm just a bit confused how the whole HAL stacks files.

I assume you already had to resolve this failing static assert before you were able to compile at all?
static_assert(COUNT(servo_delay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long.");

I didnt have any error messages about that.

Here are the files you asked for, and the pins files.

Configuration.h.txt
Configuration_adv.h.txt
pins_BTT_SKR_PRO_V1_1.h.txt

[sjasonsmith]

Are you building against the latest state of the 2.0 or bugfix-2.0.x branch? I see some indications in your configuration files that they are pretty old. Your config files are missing some changes (in unrelated features) that happened before people put a lot of effort into the SKR Pro a couple months ago.

[MangaValk]

commit bb5717c on bugfix-2.0.x branch.

I merge the config files myself, maybe some things are missing but it should all be working from what i can see.
i was going to redo the config from scratch soon, but i was trying to figure out why servo wasnt working first.

[MangaValk]

last thing i tried was change the move function in HAL_STM32/Servo.cpp to this:

void libServo::move(const int value) {
  constexpr uint16_t servo_delay[] = SERVO_DELAY;
  //static_assert(COUNT(servo_delay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long.");
  if (super::attach(servoPin[servoIndex]) >= 0) {
    super::write(value);
    safe_delay(servo_delay[servoIndex]);
    #if ENABLED(DEACTIVATE_SERVOS_AFTER_MOVE)
      super::detach();
    #endif
  }
}

but still if i try M280 p2 it only affects servo0.

[sjasonsmith]

I think your code change ends up effectively the same. I see a little risk in this class because there is some function hiding occurring with those attach() functions, which in the right conditions might lead to the incorrect version being called.

I don't really spot anything in the code that is wrong, but I'm not able to actually try anything on hardware at the moment. If you or someone else hasn't solved it by the end of next week I could help look into it further.

[MangaValk]

After spending way more hours i reached the limit of my know-how.
The only thing i can think of is that there is something going wrong in the attach function or somehow the servo array in ./src/module/servo.cpp all end being the same instance.
But after trying many different fixes, and even static attach hacks, i can't get it to work.

I hope someone can give some input that guides me in the right direction.

Edit:
So i remade my config from the ground up, now somehow i have the same behavior but instead of all on servo 0 they are all on servo 1 now. (using 2 servos)

[MangaValk]

So i got it to work by changing uint8_t ServoCount = 0; to static in the stm32 library and putting uint8_t servoIndex; to public.
And then removing uint8_t servoIndex; inside ./HAL_STM32/Servo.h.

Now 2 servos work.

But when i setup the toolchanger, the moment marlin goes trough the toolchange code the pin seems to go floating. (and bltouch stops working)

[MangaValk]

So second problem is a timer issue, swapped the TIMER_TONE and TIMER_SERVO timers and now servo keeps working during toolchange.

new variant.h file

#define TIMER_TONE              TIM2
#define TIMER_SERIAL            TIM7

// Do not use basic timer: OC is required
#define TIMER_SERVO             TIM6  //TODO: advanced-control timers don't work

[thinkyhead]

Merged the patch, so we'll find out pretty quickly if it has any conflicts.