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

Implement lights using raspi gpio #328

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

Implement lights using raspi gpio #328

wants to merge 2 commits into from

Conversation

ConnorWhalen
Copy link
Contributor

Addresses #327

The implementation is based on how the motors are implemented:
DigitalLight -> Motor
Light -> PWM
List -> Maestro
RPiGPIOLight -> MaestroChannel

@ConnorWhalen
Copy link
Contributor Author

ConnorWhalen commented Jun 20, 2017

From CircleCI:

    java.lang.AssertionError: expected:<616.0> but was:<617.0>
        at org.junit.Assert.fail(Assert.java:88)
        at org.junit.Assert.failNotEquals(Assert.java:834)
        at org.junit.Assert.assertEquals(Assert.java:553)
        at org.junit.Assert.assertEquals(Assert.java:683)
        at com.easternedgerobotics.rov.fx.ViewLoaderPrefsTest.doesResetSizeLocation(ViewLoaderPrefsTest.java:82)

This looks like a weird failure, I'm going to kick off another CI build

@cal-pratt
Copy link
Contributor

I would just delete the old light classes completely. They don't work so there's not any value in keeping them. As an aside I would probably copy the abstract map computeIfAbsent implementation instead of just hoping the list is ordered correctly once its passed to the Rov ctor. I don't really like the abstract list implementation, but maybe next year you could make a factory classes for creating I2C/Maestro/GPIO objects.

Other than that 👍 Test it on the rov then ship it.

@ConnorWhalen
Copy link
Contributor Author

FYI The old light classes have already been deleted

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

Successfully merging this pull request may close these issues.

2 participants