search

androidthings / contrib-drivers

Open source peripheral drivers
Apache License 2.0
560 stars 174 forks source link

First version of a driver for WS2801 #78

Closed sistr22 closed 6 years ago

googlebot commented 6 years ago

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).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

sistr22 commented 6 years ago

I signed it !

On Fri, Nov 10, 2017, 17:25 googlebot notifications@github.com 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 https://github.com/androidthings/contrib-drivers/pull/78#issuecomment-343535101, or mute the thread https://github.com/notifications/unsubscribe-auth/AEXm6afreu6CqtNgwKUjlLghEWqS9Zypks5s1IccgaJpZM4QZ3rm .

-- Damien SWE @ Google http://www.opengl-tutorial.org/ https://whiteseeker.github.io/

googlebot commented 6 years ago

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. 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.

proppy commented 6 years ago

It'd be nice to consolidate with #11 and #76.

Also see the design discussion in https://github.com/androidthings/contrib-drivers/issues/14 where we discuss how we can converge the API with what we already have for apa102 LEDs. https://github.com/androidthings/contrib-drivers/blob/master/apa102/src/main/java/com/google/android/things/contrib/driver/apa102/Apa102.java

sistr22 commented 6 years ago

I've taken a look to both driver. Generally speaking, I like "lightweight" driver as much as possible. The more code a driver has the more likely a bug will arise and it'll be harder to debug. I also like the idea of "1 hardware, 1 driver" but I understand that many dev prefer to consolidate code to avoid duplication, I think both approach have pros & cans and are viable. Seeing a small driver, easily readable, also avoid other potential contributor to be scare to write their own driver for a new hardware.

sistr22 commented 6 years ago

I changed my driver too much (now most of it is in C++ using JNI) to update the pull request and fix the remark :/

proppy commented 6 years ago

@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.

sistr22 commented 6 years ago

It actually has worse perf due to the JNI calls I guess (which is unfortunate). But it's easier to maintain for me, and the rest of my code is all C (to be cross platform, the project I'm working on initially started on RasberryPi). It would de nice to have a high perf C++ api to write driver (like the spi class) to make it a better experience porting project from arduino / rasberry pi or supporting all platforms. The driver implementation would still be different on each platform (but hopefully the header could be the same), but everything else would be written in C/C++ and be cross platform.