First version of a driver for WS2801 #78
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it !
On Fri, Nov 10, 2017, 17:25 googlebot ***@***.***> wrote:
Thanks for your pull request. It looks like this may be your first
contribution to a Google open source project. Before we can look at your
pull request, you'll need to sign a Contributor License Agreement (CLA).
📝 *Please visit https://cla.developers.google.com/
<https://cla.developers.google.com/> to sign.*
Once you've signed, please reply here (e.g. I signed it!) and we'll
verify. Thanks.
------------------------------
- If you've already signed a CLA, it's possible we don't have your
GitHub username or you're using a different email address. Check your
existing CLA data <https://cla.developers.google.com/clas> and verify
that your email is set on your git commits
<https://help.github.com/articles/setting-your-email-in-git/>.
- If your company signed a CLA, they designated a Point of Contact who
decides which employees are authorized to participate. You may need to
contact the Point of Contact for your company and ask to be added to the
group of authorized contributors. If you don't know who your Point of
Contact is, direct the project maintainer to go/cla#troubleshoot.
- In order to pass this check, please resolve this problem and have
the pull request author add another comment and the bot will run again.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#78 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEXm6afreu6CqtNgwKUjlLghEWqS9Zypks5s1IccgaJpZM4QZ3rm>
.
--
Damien
SWE @ Google
http://www.opengl-tutorial.org/
https://whiteseeker.github.io/
|
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
It'd be nice to consolidate with #11 and #76. Also see the design discussion in #14 where we discuss how we can converge the API with what we already have for apa102 LEDs. |
|
I've taken a look to both driver. |
| spi_device.setDelay(1000); | ||
| } | ||
|
|
||
| public void SetColor(int position, int red, int green, int blue) throws IOException { |
There was a problem hiding this comment.
google code conventions state methods should start with a lowercase letter
| public class Ws2801 { | ||
| private SpiDevice spi_device; | ||
| private byte[] data; | ||
| private int nb_leds; |
There was a problem hiding this comment.
if you call this numberOfLeds it makes the code much more understandable
|
|
||
| public void SetColor(int position, int color) { | ||
| if(position > nb_leds) | ||
| return; |
There was a problem hiding this comment.
might be better to throw an exception so a user gets instant feedback that they have done something unexpected
| SetColor(position, Color.rgb(red, green, blue)); | ||
| } | ||
|
|
||
| public void SetColor(int position, int color) { |
There was a problem hiding this comment.
What about exposing a write(android.graphics.Color[]) method like the apa driver instead?
Even if we end up with distinct drivers for each variant of LED strips, it would be nice if they were sharing some convention.
|
I changed my driver too much (now most of it is in C++ using JNI) to update the pull request and fix the remark :/ |
|
@WhiteSEEKER Do you see improved performance? I wouldn't expect much since the NDK API calls for Peripheral manager still go thru the same IPC route as the Java one. |
|
It actually has worse perf due to the JNI calls I guess (which is unfortunate). |
No description provided.