Closed mikeshub closed 5 years ago
p3p
commented
5 years ago The LPC176x build doesn't use the official library it uses my fork, it also shouldn't matter that it only supports 8bit pin numbers but I'l look into it, does it just not work or is there an error?
mikeshub
commented
5 years ago There is a warning along the lines of the value is over 256 so it will be changed, then the lights don't work. Looks like my PR is going to get included and would have been already but for an oops on my part. I didn't notice which one Marlin uses and just assumed it was the Adafruit one for LPC. Either way it's an easy fix to just make the pin widths be 16 instead of 8.
p3p
commented
5 years ago An official Arduino library shouldn't have support an quirk of my LPC176x Arduino framework so I would prefer that is wasn't merged.. As the official lib doesn't support the LPC176x anyway its the wrong place to be targeted.
https://github.com/p3p/Adafruit_NeoPixel is used by Marlin for LPC176x.
mikeshub
commented
5 years ago OK so will you please fix your code then?
EDIT: Actually I'm going to add this to the adafruit code and make another PR. Having the code all be in one place is ideal I think.
OK I added the code to the Adafruit code and made a PR. I just tried it on my SKR 1.3 setup and it works just fine off pin 1_13 using this code:
https://github.com/mikeshub/Adafruit_NeoPixel/commits/32-bit-fix
I advocate that if Adafruit includes the PR I just made we set the platformio.ini library to the official one. Nice work on your fix for this BTW.
p3p
commented
5 years ago I kept my LPC176x support out of the official lib on purpose because my LPC176x Arduino Framework is not finalised, if it ever will be, I didn't intend for it to be a general Arduino compatible framework when I was adding LPC176x support to Marlin it just moved in that direction so its a bit of a mess and has incompatibilities.
The unfinished nature of the framework is why you are having the issue with the pin numbers, at the moment it is better we keep the LPC176x support out of the official library, so a PR into my fork would have been preferred.
mikeshub
commented
5 years ago Whichever way is fine by me. Let's just one way or the other get the width of the pin vars set to 16 and call it a day.
mikeshub
commented
5 years ago Looks like PR was included. I can make a PR later to update the library in the .ini file if you think that going with the official is the way to go and if not please update your code.
The Arduino framework question is almost a philosophical one. I would argue that Marlin has added the LPC1768 framework to arduino and official support just hasn't caught up yet. If it looks like a duck, walks like a duck, doesn't have a single printf and uses digitalWrite calls, then it's a duck.
p3p
commented
5 years ago I've reset my fork to upstream but I'l leave it in platformio.ini for the time being as I may find time to work on my Arduino framework and updates will require changes to it.
I'm a little annoyed that this got merged into the official lib as it complicates things, and its not particularly good form to just copy and paste code out of someone else's work for a PR even when it is open source, but it's done now.
mikeshub
commented
5 years ago A prudent choice. Looks like the STM32F4 code is having a similar issue.
mylife4aiurr
commented
5 years ago Just saying thanks here. As of at least 52a7a8c neo_pixel was unreliable, unusable on skr 1.3 lpc176x.
Not accurate colors, wrong pixel placement. Same M150 commands had different results. I thought it was me. Tried different pins Tried adding a resistor to neo_pixel pin Played with different voltages for leds
Updated to 2971b48a128662ab2f50a0a238aedfb64ec785d9 today. Rainbow sequence all leds had same accurate color Color test menu produced accurate colors Print events worked with correct colors and correct amount of pixels
github-actions[bot]
commented
4 years ago 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.
Description
LPC1768 and neopixels has an issue where the value to the pin that is passed to the constructor is greater than 255. This causes issues with the LPC1768.
Steps to Reproduce
On an LPC1768 based board try using pin P1_31 as the Neopixel pin.
Additional Information
This can be handled one of two ways. First off there isn't a way to achieve this by just extending the class which was my first plan. So we can either wait for Adafruit to handle this PR then once they get around to it we go in an change the lib_deps to all the working version.
https://github.com/adafruit/Adafruit_NeoPixel/pull/197
Or if we don't feel like waiting on Adafruit the Neopixel libraries in the lib_deps can be changed to the branch I made the PR with.