search

arduino-libraries / Servo

Servo Library for Arduino
http://arduino.cc/
GNU Lesser General Public License v2.1
252 stars 261 forks source link

Incomplete samd51 implementation #21

Open Silvanosky opened 5 years ago

Silvanosky commented 5 years ago

Hi,

The implementation for the samd servo library doesn't work on samd51.

The syncbusy bit and clock management are different on this new chip so the code here need to be adapted for samd51.

For example when I try to compile the sweep example for the Feather M4 I get this :


Build options changed, rebuilding all
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp: In function 'void Servo_Handler(timer16_Sequence_t, Tc*, uint8_t, uint8_t)':
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:64:9: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
         WAIT_TC16_REGS_SYNC(tc)
         ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:81:9: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
         WAIT_TC16_REGS_SYNC(tc)
         ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:84:9: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
         WAIT_TC16_REGS_SYNC(tc)
         ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:91:9: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
         WAIT_TC16_REGS_SYNC(tc)
         ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:99:9: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
         WAIT_TC16_REGS_SYNC(tc)
         ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp: In function 'void resetTC(Tc*)':
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:112:5: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
     WAIT_TC16_REGS_SYNC(TCx)
     ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:116:5: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
     WAIT_TC16_REGS_SYNC(TCx)
     ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp: In function 'void _initISR(Tc*, uint8_t, uint32_t, IRQn_Type, uint8_t, uint8_t)':
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:123:11: error: 'struct Gclk' has no member named 'CLKCTRL'
     GCLK->CLKCTRL.reg = (uint16_t) (GCLK_CLKCTRL_CLKEN | GCLK_CLKCTRL_GEN_GCLK0 | GCLK_CLKCTRL_ID(gcmForTimer));
           ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:123:37: error: 'GCLK_CLKCTRL_CLKEN' was not declared in this scope
     GCLK->CLKCTRL.reg = (uint16_t) (GCLK_CLKCTRL_CLKEN | GCLK_CLKCTRL_GEN_GCLK0 | GCLK_CLKCTRL_ID(gcmForTimer));
                                     ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:123:58: error: 'GCLK_CLKCTRL_GEN_GCLK0' was not declared in this scope
     GCLK->CLKCTRL.reg = (uint16_t) (GCLK_CLKCTRL_CLKEN | GCLK_CLKCTRL_GEN_GCLK0 | GCLK_CLKCTRL_ID(gcmForTimer));
                                                          ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:123:110: error: 'GCLK_CLKCTRL_ID' was not declared in this scope
     GCLK->CLKCTRL.reg = (uint16_t) (GCLK_CLKCTRL_CLKEN | GCLK_CLKCTRL_GEN_GCLK0 | GCLK_CLKCTRL_ID(gcmForTimer));
                                                                                                              ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:124:18: error: 'struct Gclk' has no member named 'STATUS'
     while (GCLK->STATUS.bit.SYNCBUSY);
                  ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:134:30: error: 'TC_CTRLA_WAVEGEN_NPWM' was not declared in this scope
     tc->COUNT16.CTRLA.reg |= TC_CTRLA_WAVEGEN_NPWM;
                              ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:141:5: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
     WAIT_TC16_REGS_SYNC(tc)
     ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:145:5: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
     WAIT_TC16_REGS_SYNC(tc)
     ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:44:60: error: 'volatile struct TC_STATUS_Type::<anonymous>' has no member named 'SYNCBUSY'
 #define WAIT_TC16_REGS_SYNC(x) while(x->COUNT16.STATUS.bit.SYNCBUSY);
                                                            ^
/home/silvanosky/Arduino/libraries/Servo/src/samd/Servo.cpp:159:5: note: in expansion of macro 'WAIT_TC16_REGS_SYNC'
     WAIT_TC16_REGS_SYNC(tc)
     ^
exit status 1
Error compiling for board Adafruit Feather M4 Express (SAMD51).
ladyada commented 5 years ago

@facchinm heya would y'all take a PR to fix this?

facchinm commented 5 years ago

Hi @ladyada , we are going to merge the PR, no problem :wink:

PaintYourDragon commented 5 years ago

I’m on it. It’s a little more involved than just the sync registers, but I should have something this afternoon.

PaintYourDragon commented 5 years ago

A pull request has been made to arduino-libraries:master. If you need it sooner than the next merge, can be downloaded here: https://github.com/PaintYourDragon/Servo

josephkapp commented 5 years ago

Thank you for the fix, this has been driving me crazy for weeks! Was about to attempt to write my own PWM class. It is compiling for me now.

knoxvillesjoker commented 5 years ago

Please note, in your existing sketch, copy the servo.h from this updated library which you will download and place in your Arduino libraries folder under servo-master, into whatever sketch you are using.

richard-pepe commented 5 years ago

I tried this, but I still get a number of compiler errors when compiling for the Adafruit ItsyBitsy M4 (SAMD51). 

In file included from /home/rlpepe/Arduino/libraries/Servo-master/src/Servo.h:67:0,
                 from /home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:22:
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void TC4_Handler()':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/ServoTimers.h:41:35: error: 'TC4' was not declared in this scope
 #define TC_FOR_TIMER1             TC4
                                   ^

I get the same for a number of other macro definitions. The list is much shorter than before updating the library, but I still can't compile successfully. Any thoughts?

knoxvillesjoker commented 5 years ago

Tc4 needs to be a declared variable?

At top add :

const int TC4;
TC4 = 4;
GMagician commented 5 years ago

Hi, since I'm working on Marlin and I integrated Grand Central M4 into it, this servo library will be very helpful if integrated into libraries (Marlin uses platformIO but I think this is not a problem). There are some news about the status of this?

richard-pepe commented 5 years ago

There seem to be a number of issues yet to resolve to support the SAMD51 (compiles fine for the SAMD21 though). Here's the complete list of compiler messages when compiling for the Adafruit ItsyBitsy M4:

/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void TC4_Handler()':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:63:86: error: invalid conversion from 'int' to 'Tc*' [-fpermissive]
     Servo_Handler(_timer1, TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, INTFLAG_BIT_FOR_TIMER_1);
                                                                                      ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:60:6: error:   initializing argument 2 of 'void Servo_Handler(timer16_Sequence_t, Tc*, uint8_t, uint8_t)' [-fpermissive]
 void Servo_Handler(timer16_Sequence_t timer, Tc *pTc, uint8_t channel, uint8_t intFlag);
      ^
In file included from /home/rlpepe/Arduino/libraries/Servo-master/src/Servo.h:67:0,
                 from /home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:22:
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void initISR(timer16_Sequence_t)':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/ServoTimers.h:48:35: error: 'ID_TC4' was not declared in this scope
 #define ID_TC_FOR_TIMER1          ID_TC4
                                   ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:251:53: note: in expansion of macro 'ID_TC_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                                                     ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/ServoTimers.h:49:35: error: 'TC4_IRQn' was not declared in this scope
 #define IRQn_FOR_TIMER1           TC4_IRQn
                                   ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:251:71: note: in expansion of macro 'IRQn_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                                                                       ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void finISR(timer16_Sequence_t)':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:263:18: error: base operand of '->' is not a pointer
     TC_FOR_TIMER1->COUNT16.INTENCLR.reg = INTENCLR_BIT_FOR_TIMER_1;
                  ^
GMagician commented 5 years ago

@richard-pepe that sound weird, I temporary added #28 in Marlin and it compile fine for samd51 (don't know if it works because I don't have proper H/W to test)

richard-pepe commented 5 years ago

I gave it a shot, but still doesn't compile.

In file included from /home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/Servo.h:67:0,
                 from /home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp:19:
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp: In function 'void TC4_Handler()':
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/ServoTimers.h:41:35: error: 'TC4' was not declared in this scope
 #define TC_FOR_TIMER1             TC4
                                   ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp:60:28: note: in expansion of macro 'TC_FOR_TIMER1'
     Servo_Handler(_timer1, TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, INTFLAG_BIT_FOR_TIMER_1);
                            ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp: In function 'void initISR(timer16_Sequence_t)':
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/ServoTimers.h:41:35: error: 'TC4' was not declared in this scope
 #define TC_FOR_TIMER1             TC4
                                   ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp:248:18: note: in expansion of macro 'TC_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                  ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/ServoTimers.h:46:35: error: 'ID_TC4' was not declared in this scope
 #define ID_TC_FOR_TIMER1          ID_TC4
                                   ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp:248:53: note: in expansion of macro 'ID_TC_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                                                     ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/ServoTimers.h:47:35: error: 'TC4_IRQn' was not declared in this scope
 #define IRQn_FOR_TIMER1           TC4_IRQn
                                   ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp:248:71: note: in expansion of macro 'IRQn_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                                                                       ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp: In function 'void finISR(timer16_Sequence_t)':
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/ServoTimers.h:41:35: error: 'TC4' was not declared in this scope
 #define TC_FOR_TIMER1             TC4
                                   ^
/home/rlpepe/Downloads/arduino-1.8.5-linux64/arduino-1.8.5/libraries/Servo/src/samd/Servo.cpp:260:5: note: in expansion of macro 'TC_FOR_TIMER1'
     TC_FOR_TIMER1->COUNT16.INTENCLR.reg = INTENCLR_BIT_FOR_TIMER_1;
     ^
knoxvillesjoker commented 5 years ago

Looks like the timer function is not properly populating. In servo.cpp comment out function void tc4_handler()

GMagician commented 5 years ago

In marlin I have done some job on it and now it should works. Maybe some hints may be got from there

richard-pepe commented 5 years ago

Not working for me. A number of things aren't defined (TC4, ID_TC4, TC4_IRQn) along with other issues:

/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void TC4_Handler()':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:60:86: error: invalid conversion from 'int' to 'Tc*' [-fpermissive]
     Servo_Handler(_timer1, TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, INTFLAG_BIT_FOR_TIMER_1);
                                                                                      ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:57:6: error:   initializing argument 2 of 'void Servo_Handler(timer16_Sequence_t, Tc*, uint8_t, uint8_t)' [-fpermissive]
 void Servo_Handler(timer16_Sequence_t timer, Tc *pTc, uint8_t channel, uint8_t intFlag);
      ^
In file included from /home/rlpepe/Arduino/libraries/Servo-master/src/Servo.h:67:0,
                 from /home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:19:
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void initISR(timer16_Sequence_t)':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/ServoTimers.h:48:35: error: 'ID_TC4' was not declared in this scope
 #define ID_TC_FOR_TIMER1          ID_TC4
                                   ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:248:53: note: in expansion of macro 'ID_TC_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                                                     ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/ServoTimers.h:49:35: error: 'TC4_IRQn' was not declared in this scope
 #define IRQn_FOR_TIMER1           TC4_IRQn
                                   ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:248:71: note: in expansion of macro 'IRQn_FOR_TIMER1'
         _initISR(TC_FOR_TIMER1, CHANNEL_FOR_TIMER1, ID_TC_FOR_TIMER1, IRQn_FOR_TIMER1, GCM_FOR_TIMER_1, INTENSET_BIT_FOR_TIMER_1);
                                                                       ^
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp: In function 'void finISR(timer16_Sequence_t)':
/home/rlpepe/Arduino/libraries/Servo-master/src/samd/Servo.cpp:260:18: error: base operand of '->' is not a pointer
     TC_FOR_TIMER1->COUNT16.INTENCLR.reg = INTENCLR_BIT_FOR_TIMER_1;
knoxvillesjoker commented 5 years ago

I am starting to think that the issues are chipset compatibility issues with the newer samd. Those functions may require cosimg updates in the libraries.

jgarbers commented 5 years ago

I grabbed the fork that @PaintYourDragon posted, and that compiled fine for me. However, it only seems to work with the first servo object defined. If, for example, I have code like this:

Servo s1;
s1.attach(3);
s1.write(100);

Servo s2;
s2.attach(4);
s2.write(100);

the servo attached to pin 3 responds appropriately and my scope shows what looks like a reasonable PWM signal, but the servo on pin 4 doesn't move and the signal on the scope is not at all the same. If I change the order, so s2 is defined before s1, then s2 works and s1 doesn't. The issue seems to be related only to the order in which the objects are instantiated, and doesn't have anything to do with what pins are in use. It also doesn't seem to matter whether I call the attach method on the first servo or not; the second servo still doesn't get the right signal.

At a suggestion from adafruit_support_bill, I've copied this comment from a post I made over on the Adafruit forum, where there's a screenshot from my scope showing the signals being generated. Happy to help troubleshoot from here if I can.

ladyada commented 5 years ago

hiya please try https://github.com/arduino-libraries/Servo/pull/34 which should compie & work with samd51g19 and j19

ladyada commented 5 years ago

multiple-servos also fixed

GMagician commented 5 years ago

Not sure if it works (I don't have a valid shield to test) and/or may be usefull. This is what I did in Marlin to support both timers:

HAL_SERVO_TIMER_ISR() {
  Tc * const tc = TimerConfig[SERVO_TC].pTimer;
  const timer16_Sequence_t timer =
    #ifndef _useTimer1
      _timer2
    #elif !defined(_useTimer2)
      _timer1
    #else
      (tc->COUNT16.INTFLAG.reg & tc->COUNT16.INTENSET.reg & TC_INTFLAG_MC0) ? _timer1 : _timer2
    #endif
  ;
  const uint8_t tcChannel = TIMER_TCCHANNEL(timer);

  if (currentServoIndex[timer] < 0) {
    #if defined(_useTimer1) && defined(_useTimer2)
      if (currentServoIndex[timer ^ 1] >= 0) {
        // Wait for both channels
        // Clear the interrupt
        tc->COUNT16.INTFLAG.reg = (tcChannel == 0) ? TC_INTFLAG_MC0 : TC_INTFLAG_MC1;
        return;
      }
    #endif
    tc->COUNT16.COUNT.reg = TC_COUNTER_START_VAL;
    SYNC(tc->COUNT16.SYNCBUSY.bit.COUNT);
  }
  else if (SERVO_INDEX(timer, currentServoIndex[timer]) < ServoCount && SERVO(timer, currentServoIndex[timer]).Pin.isActive)
    digitalWrite(SERVO(timer, currentServoIndex[timer]).Pin.nbr, LOW);      // pulse this channel low if activated

  // Select the next servo controlled by this timer
  currentServoIndex[timer]++;

  if (SERVO_INDEX(timer, currentServoIndex[timer]) < ServoCount && currentServoIndex[timer] < SERVOS_PER_TIMER) {
    if (SERVO(timer, currentServoIndex[timer]).Pin.isActive)                // check if activated
      digitalWrite(SERVO(timer, currentServoIndex[timer]).Pin.nbr, HIGH);   // it's an active channel so pulse it high

    tc->COUNT16.CC[tcChannel].reg = getTimerCount() - (uint16_t)SERVO(timer, currentServoIndex[timer]).ticks;
  }
  else {
    // finished all channels so wait for the refresh period to expire before starting over
    currentServoIndex[timer] = -1;   // this will get incremented at the end of the refresh period to start again at the first channel

    const uint16_t tcCounterValue = getTimerCount();

    if ((TC_COUNTER_START_VAL - tcCounterValue) + 4UL < usToTicks(REFRESH_INTERVAL))  // allow a few ticks to ensure the next OCR1A not missed
      tc->COUNT16.CC[tcChannel].reg = TC_COUNTER_START_VAL - (uint16_t)usToTicks(REFRESH_INTERVAL);
    else
      tc->COUNT16.CC[tcChannel].reg = (uint16_t)(tcCounterValue - 4UL);               // at least REFRESH_INTERVAL has elapsed
  }
  if (tcChannel == 0) {
    SYNC(tc->COUNT16.SYNCBUSY.bit.CC0);
    // Clear the interrupt
    tc->COUNT16.INTFLAG.reg = TC_INTFLAG_MC0;
  }
  else {
    SYNC(tc->COUNT16.SYNCBUSY.bit.CC1);
    // Clear the interrupt
    tc->COUNT16.INTFLAG.reg = TC_INTFLAG_MC1;
  }

SYNC(a) is a define that just do while(a) #define TIMER_TCCHANNEL(t) ((t) & 1) because I tied timerN with channel

ladyada commented 5 years ago

we're doing something similar here, we wanted to be as close to the samd21 implementation as possible

GMagician commented 5 years ago

yes but SAMD51 CC0 and CC1 share the same timer count, so something different must be done

ladyada commented 5 years ago

hiya please explain in more detail. we're using all of TC0, for now just CC0 but it doesnt matter because CC1 wont be usable by anything else anyways. please reference the PR - we want to make this as similar as possible to the SAMD21 code so that means not refactoring the ISR

GMagician commented 5 years ago

I'll try to explain, but it's not easy due to the fact English is not my natural language. Code should support 2 timers (timer1 and timer2), both share TC0 and use both TC0 channels (0 and 1) these compare COUNT to CC0 and CC1. Once CC0 terminates its job COUNT is reset but since also CC1 is using the same COUNT value you will change its timing. AFAIK samd21 has separate counts for channels

ladyada commented 5 years ago

OK i know what you're saying - it seems SAMD21 has only one COUNT per timer as well?

Table 30-5. Register Summary – 16-bit Mode

image image

GMagician commented 5 years ago

oh yes it seems..but how can it works on samd21?

ladyada commented 5 years ago

it doesnt https://github.com/arduino-libraries/Servo/blob/master/src/samd/ServoTimers.h#L37

GMagician commented 5 years ago

oh how stupid I am...I read it but I was thinking only at samd51...but 21 is also samd...