Legion2 / CorsairLightingProtocol

Control LEDs connected to an Arduino with iCUE, create an unofficial Corsair iCUE compatible Arduino LED controller.
Apache License 2.0
499 stars 64 forks source link

Use of PWMFan causes device to disconnect/freeze when setting RGB on Arduino Pro Micro clone #345

Closed xCuri0 closed 1 year ago

xCuri0 commented 1 year ago

Describe the bug Even just declaring a PWMFan causes my Arduino Pro Micro to disconnect or freeze when RGB is set. I also tried commenting out the timer related code in it and it made no difference.

To Reproduce Steps to reproduce the behavior:

  1. Use Commander Pro Sketch and upload it
  2. Set RGB, I'm using OpenRGB haven't tried iCUE
  3. Device will either disconnect or stop responding to RGB values

Expected behavior I expect it to not crash when setting RGB, if I use the Lighting Node Pro sketch or remove PWMFan from Commander Pro it works fine. Even without the use of addFan and/or updateFans it will cause this behaviour.

System (please complete the following information):

I currently only have an ARGB fan connected but I do plan on adding a 4 pin fan once I figure this out

Code changes None, I have to modify Commander Pro to remove PWMFan if I want to use it without crashing.

Legion2 commented 1 year ago

Calling the PWMFan constructor configures timer registers of the arduino to support higher refresh rates. However these settings has only been tested on arduino mega. Maybe you can check if these settings are correct or need to be changed for Arduino Pro Micro, so we can add support for it.

https://github.com/Legion2/CorsairLightingProtocol/blob/c0f4fd1e29a8011a55153514e37cd5633465df35/examples/CommanderPRO/PWMFan.cpp#L21-L46

xCuri0 commented 1 year ago

@Legion2 That's what I thought, but with all the timer related code commented out I still get the same behaviour

PWMFan::PWMFan(uint8_t pwmPin, uint16_t minRPM, uint16_t maxRPM) : pwmPin(pwmPin), minRPM(minRPM), maxRPM(maxRPM) {
  pinMode(pwmPin, OUTPUT);
  analogWrite(pwmPin, 0);
  /*
  switch (digitalPinToTimer(pwmPin)) {
    case TIMER0B: /* 3 
      Serial.println(F("Pin not supported as PWM fan pin"));
      Serial.println(F("We don't want to mess up Arduino time functions"));
      break;
    case TIMER3A: /* 5 
      TCCR3B = (TCCR3B & B11111000) | 0x01;
      break;
    case TIMER4D: /* 6 
      // PLLFRQ = (PLLFRQ & B11001111) | (0x03 << PLLTM0);
      TCCR4B = (TCCR4B & B11110000) | 0x01;
      break;
    case TIMER1A: /* 9 
      TCCR1B = (TCCR1B & B11111000) | 0x01;
      break;
    case TIMER1B: /* 10 
      TCCR1B = (TCCR1B & B11111000) | 0x01;
      break;
    default:
      Serial.println(F("Pin not supported as PWM fan pin"));
      break;
  }

  */
}

I thought it could be an issue with not getting built but if I add a print there it shows

xCuri0 commented 1 year ago

So if I set OpenRGB to only use Channel 1 (using size 16, did same for 2) it works fine using unmodified Commander Pro example, is this a memory issue or something similar ?

EDIT: Reducing CHANNEL_LED_COUNT appears to fix this

EDIT 2: Seems like 86 is the maximum amounts of LEDs Pro Micro can handle on both channels, could be less when actually controlling the fans tho

I'll try to fix Pro Micro PWM pins once I get a fan connected

xCuri0 commented 1 year ago

I'm closing since it appears to be just a memory issue due to Pro Micro having less, maybe a comment could be added saying that 96 doesn't work on Pro Micro ?

Legion2 commented 1 year ago

Thanks for your investigation, a comment would be nice, feel free to open a PR.

xCuri0 commented 12 months ago

even 88 might be too high, 48 seems safe and 56 may work