★ wanayoo — archive 1999 https://github.com/ev3dev-lang-java/ev3dev-lang-java/pull/540Nouvelle recherche | Portail wanayoo
Skip to content

/ ev3dev-lang-java

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

Jump to bottom

Code fixes for Feature/#498 #540

Merged
merged 14 commits into from Oct 30, 2018
Merged

Code fixes for Feature/#498 #540

merged 14 commits into from Oct 30, 2018

Conversation

@JakubVanek
Copy link
Contributor

@JakubVanek JakubVanek commented Oct 29, 2018

This resolves all my objections I've had.
JakubVanek added 7 commits Oct 29, 2018
@JakubVanek
Add value read function to EV3DevDevice
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
0a9f13e
@JakubVanek
Refactor BaseSensor
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
b709345 
- removed VALUEn constants
- cleaned up code a little bit
- removed integer-based switchMode
- made string-based switchMode public
- removed saving of current sensor mode - we read always from the kernel now
@JakubVanek
Reimplement GenericMode w/ BaseSensor functions
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
e7d91b8 
- reading now depends on BaseSensor
- switchMode is called on sensor before reading data
- merged all sensor cases into one reading loop
@JakubVanek
Port EV3 & Mindsensors sensors to the new GenericMode
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
a23382d
@JakubVanek
Fix off-by-one error in EV3IRSensor
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
2475515 
- sensor mode 3 doesn't not exist, but 2 does
@JakubVanek
Update sensor unit tests to match GenericMode work
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
71f7b96
@JakubVanek
Refactor fake sensors - lower code duplication
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
Loading status checks…
bba052d
@JakubVanek
Fix misleading comment about switchMode in GenericMode
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
Loading status checks…
e8d1e66
@JakubVanek
JakubVanek reviewed Oct 29, 2018
View changes
src/main/java/ev3dev/sensors/ev3/EV3ColorSensor.java Outdated Show resolved Hide resolved
JakubVanek added 4 commits Oct 29, 2018
@JakubVanek
Fix floodlight changing in EV3ColorSensor
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
Loading status checks…
8bc6e1a
@JakubVanek
Rename 4 to IR_CHANNELS in EV3IRSensor
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
Loading status checks…
5fccf77
@JakubVanek
Follow argument description in EV3IRSensor#getRemoteCommands
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
Loading status checks…
74325b4
@JakubVanek
Extract kernel sensor mode read/write to its own function
Verified
This commit was signed with a verified signature.
JakubVanek Jakub Vaněk
GPG key ID: CCDE818F3F894D36 Learn about signing commits
Loading status checks…
5476611
@JakubVanek JakubVanek requested a review from jabrena Oct 29, 2018
@JakubVanek JakubVanek added this to the v0.9.0 milestone Oct 29, 2018
@JakubVanek
Copy link
Contributor Author

@JakubVanek JakubVanek commented Oct 29, 2018 •
edited

Hmmm, BaseSensor#setCurrentMode()/getCurrentMode()/fetchSample() are now independent from the system sensor mode.
@jabrena
Copy link
Member

@jabrena jabrena commented Oct 29, 2018

Tomorrow, I will check and I will merge in the branch
@jabrena
Copy link
Member

@jabrena jabrena commented Oct 30, 2018

I have reviewed the changes in Github. I liked the refactoring in both sides: Classes & Tests.

Maybe we mixed the refactoring in the tests with the changes in the objects for sensors.
It is better to do one change per time, if we change sensor class, don't change tests. It is important for the future to reduce the complexity of the analysis.

Later, I will download the branch to review the changes in the sensor classes.

Juan Antonio
@JakubVanek
Copy link
Contributor Author

@JakubVanek JakubVanek commented Oct 30, 2018

Oki, thanks for the review.
Yup, looking back the fake sensor refactoring is independent from the work done on sensors. I'm thinking about doing the same thing to the motor classes, but that has even less to do with sensor classes.
@jabrena
Copy link
Member

@jabrena jabrena commented Oct 30, 2018

For the motor in another moment, put the focus in the issue. Later we will refactor :)
jabrena added 2 commits Oct 30, 2018
@jabrena
Using the methods from SysFs
Loading status checks…
e5fc737
@jabrena
Moving switch execution to the right place
Loading status checks…
dbdec95
@jabrena
jabrena approved these changes Oct 30, 2018
View changes
Copy link
Member

@jabrena jabrena left a comment

Reviewed
src/main/java/ev3dev/hardware/EV3DevDevice.java Outdated Show resolved Hide resolved
src/main/java/ev3dev/sensors/ev3/EV3TouchSensor.java Show resolved Hide resolved
src/main/java/ev3dev/sensors/mindsensors/AbsoluteIMU.java Show resolved Hide resolved
src/test/java/ev3dev/sensors/ev3/EV3ColorSensorTest.java Show resolved Hide resolved
src/test/java/fake_ev3dev/BaseElement.java Show resolved Hide resolved
src/test/java/fake_ev3dev/ev3dev/sensors/FakeLegoSensor.java Show resolved Hide resolved
src/test/java/fake_ev3dev/ev3dev/sensors/FakeLegoSensor.java Show resolved Hide resolved
src/main/java/ev3dev/sensors/GenericMode.java Outdated Show resolved Hide resolved
@jabrena jabrena merged commit 2f011a6 into feature/#498 Oct 30, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@jabrena jabrena deleted the feature/#498_review branch Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jabrena jabrena

Assignees

@jabrena jabrena

Labels
None yet
Projects
None yet
Milestone
v0.9.0
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@JakubVanek @jabrena
You can’t perform that action at this time.