Skip to content

Fixed simplecyclic test#540

Merged
felixdivo merged 4 commits intohardbyte:developfrom
bhass1:3.1.1-cyclic_test_fix
Apr 4, 2019
Merged

Fixed simplecyclic test#540
felixdivo merged 4 commits intohardbyte:developfrom
bhass1:3.1.1-cyclic_test_fix

Conversation

@bhass1
Copy link
Copy Markdown
Contributor

@bhass1 bhass1 commented Apr 1, 2019

Originally, the testcase would fail because timestamps would not match.
This was due to a Can.Message being initialized had a timestamp
value of 0, while a recv'd message had a virtual timestamp. When compared
they never matched.

Modified testcase now allows for some variation (0.016 s) in the timestamp
for a 0.01 s periodic message and compares the last message with the next
last message.

Originally, the testcase would fail because timestamps would not match.
This was due to a Can.Message being initialized had a timestamp
value of 0, while a recv'd message had a virtual timestamp. When compared
they never matched.

Modified testcase now allows for some variation (0.016 s) in the timestamp
for a 0.01 s periodic message and compares the last message with the next
last message.
@bhass1 bhass1 changed the base branch from release-3.1.1 to develop April 1, 2019 15:24
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2019

Codecov Report

Merging #540 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #540   +/-   ##
========================================
  Coverage    64.44%   64.44%           
========================================
  Files           63       63           
  Lines         5651     5651           
========================================
  Hits          3642     3642           
  Misses        2009     2009

@felixdivo felixdivo added this to the 3.2 Release milestone Apr 4, 2019
@felixdivo
Copy link
Copy Markdown
Collaborator

But this does not check whether the message that should be sent was actually sent. Could we add that back again, while ignoring the timestamp on that comparison only?

@bhass1
Copy link
Copy Markdown
Contributor Author

bhass1 commented Apr 4, 2019

Sure, how about by zeroizing the timestamp on the recv'd message before comparison? I didn't see an easier way to do it because of how the "allowed_timestamp_delta" works.

@felixdivo
Copy link
Copy Markdown
Collaborator

Hm yeah, that should do. I didn't implement it more flexible than that. ;)

@felixdivo
Copy link
Copy Markdown
Collaborator

But you can leave the current test in as well.

@bhass1
Copy link
Copy Markdown
Contributor Author

bhass1 commented Apr 4, 2019

But you did implement it which is saying more than most! Will push shortly. I'm having issues with running the tests off the develop branch in Windows which is why I was contributing from 3.1.1. I should probably open an issue ticket regarding it...

@felixdivo
Copy link
Copy Markdown
Collaborator

If this is solving the only problem there was, you do not need to create a ticket for it.

@bhass1
Copy link
Copy Markdown
Contributor Author

bhass1 commented Apr 4, 2019

This PR solves a different problem that was present on release-3.1.1 and hasn't been fixed yet. Once this PR is merged, release-3.1.1 passes all tests on my machine.

I created a new issue (#544) addressing the testing issues present in the develop branch.

@felixdivo felixdivo merged commit b1d64cf into hardbyte:develop Apr 4, 2019
@bhass1 bhass1 deleted the 3.1.1-cyclic_test_fix branch April 15, 2019 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants