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

Slipping detection #193

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

Slipping detection #193

wants to merge 4 commits into from

Conversation

alvynw
Copy link
Contributor

@alvynw alvynw commented Mar 17, 2018

No description provided.

@alvynw alvynw requested a review from shadaj March 17, 2018 04:08
@codecov
Copy link

codecov bot commented Mar 17, 2018

Codecov Report

Merging #193 into master will increase coverage by 0.32%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
+ Coverage   71.65%   71.97%   +0.32%     
==========================================
  Files          84       85       +1     
  Lines        1623     1638      +15     
  Branches      151      151              
==========================================
+ Hits         1163     1179      +16     
+ Misses        460      459       -1
Impacted Files Coverage Δ
...potassium/commons/cartesianPosition/Position.scala 0% <ø> (ø)
...robotics/potassium/commons/position/Slipping.scala 100% <100%> (ø)
...om/lynbrookrobotics/potassium/streams/Stream.scala 92.8% <100%> (+0.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7456ecc...f5f1b5d. Read the comment docs.

implicit val (clock, triggerClock) = ClockMocking.mockedClockTicker
val period = Milliseconds(5)

test("left and right slipping") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include more descriptive test names that describe the specific parameters and conditions you are testing. For example, "test no slipping detected with no linear or angular acceleration

val angularVelocity: Stream[AngularVelocity] = Stream.periodic(period)(RadiansPerSecondSquared(5)).integral
val linearAcceleration: Stream[Acceleration] = Stream.periodic(period)(FeetPerSecondSquared(5))
val leftEncoderVelocity: Stream[Velocity] = Stream.periodic(period)(FeetPerSecondSquared(2)).integral
val rightEncoderVelocity: Stream[Velocity] = Stream.periodic(period)(FeetPerSecondSquared(9.5)).integral
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use smaller, easier to compute numbers to avoid mistakes in unit tests.


triggerClock.apply(period)
triggerClock.apply(period)
triggerClock.apply(period)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you call apply so many times?

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.

3 participants