Gurjot95 / AlienFX-SDK

Better AlienFX/LightFX SDK than Dell official's one without any limitations.
30 stars 8 forks source link

New devices and functions support. #6

Closed T-Troll closed 4 years ago

T-Troll commented 4 years ago
Gurjot95 commented 4 years ago

Hey, I just had a quick look at the commit, even though I don't own any M series to test out myself but it seems when adding support for newer models, you removed support for older models as you overwrote the device indexes. It would be better if you select the Indexes according to device detected. If it's legacy models, use older index otherwise new.

Thanks for your contribution though. I am sure many users will be happy with this support. Cheers.

Gurjot95 commented 4 years ago

also, why is CPP file showing as a Bin file in your commits? I can't see code difference.

T-Troll commented 4 years ago

Sorry for delayed reply, i was out of reach for a couple of days.

  1. No, i don't change indexes, just mark the difference for me in comments. I know it's also requires a mask for old device (but not for new ones), so better keep it intact for old. Let me check, and i rollback any changes here if i do some (except comments).

  2. Yes, i see... Looks strange, in my repo it's a text file (BTW, you can check branch merge there - it's the same). Looks like Github bug. I have actions support now, so let me make another PR, and if it still be binary - i contact github support.

T-Troll commented 4 years ago
  1. Both CPP files in YOU repository are in UTF-16. Git can't handle them as text and merge. I fix this in mine, but you should fix it in yours to merge (open and "save as" with encoding change). I'll create new merge request with this changes.