Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor fixes in 1.9and port to ESP8266 #16

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

akastulin
Copy link

Hi Guys,
I have a version based on v1.9 that is working with ESP8266, Plus I fixed couple of bugs related to PIC.
Would you like me to upload my code?
By the way this is the first time I'm using github, so thank you for your patience....
PS2X_lib.zip

Regards

RobotShop and others added 7 commits July 16, 2013 15:04
Added an example for the Arduino Mega 2560 and the Lynxmotion PS2 v4
controller.
This example uses the following pins:
(PS2 receiver adaptor board) -> (Arduino Mega 2560)
GND -> GND
5V  -> 5V
CLK -> 22
CMD -> 23
ATT -> 24
DAT -> 25
@leesei
Copy link

leesei commented Jul 10, 2019

We usually do only one thing in a PR.
You did these in this PR:

  1. updated timing (is this update only applicable to Lynxmotion controller?)
  2. added LICENSE
  3. renamed .pde to .ino
  4. updated pin in example
  5. added Mega example
  6. added a .zip file

As a fellow OSS contributor, here are my 2 cents:

  • I think 2,3 is long due and okay to be bundled with some PR, but personally I would open new PR for 2
  • I don't see how ESP8266 is supported (via pin update?), and there's already merged PR ESP8266 support #9
  • please explain if the timing update is specific to Lynxmotion controller
  • what is the bugfix actually?
  • we would not commit binary (.zip) to Arduino library, the zip is generated by GitHub's "Download ZIP" link
  • we usually merge commits before submitting PR
  • you should rebase your branch to master and resolve the conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants