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

Update COBC-EDU protocol #169

Merged
merged 5 commits into from
Dec 10, 2023
Merged

Update COBC-EDU protocol #169

merged 5 commits into from
Dec 10, 2023

Conversation

elenajochum
Copy link
Contributor

Fixes #164

@elenajochum elenajochum requested a review from PatrickKa December 1, 2023 22:35
@PatrickKa PatrickKa force-pushed the queueId-to-timestamp branch from 905c387 to 4d47dda Compare December 4, 2023 11:02
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (84fda26) 81.81% compared to head (b62a297) 81.69%.

Files Patch % Lines
Sts1CobcSw/Edu/EduMock.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
- Coverage   81.81%   81.69%   -0.13%     
==========================================
  Files          16       16              
  Lines         451      448       -3     
==========================================
- Hits          369      366       -3     
  Misses         82       82              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@PatrickKa PatrickKa left a comment

Choose a reason for hiding this comment

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

I just realized that I made a mistake when I explained the issue to you 🙈. The queueId should not just blindly be replaced with a timestamp. The queueId should be removed from the QueueEntry because the programId together with the startTime already uniquely identify different program runs. The commands to the EDU should then use this startTime instead of the queueId. It doesn't help that the EDU PDD calls our startTime Timestamp. So, basically queueId should be replaced with startTime, except when startTime is already present, like in the QueueEntry. Then queueId should be removed.

@PatrickKa PatrickKa self-requested a review December 10, 2023 17:26
@PatrickKa PatrickKa force-pushed the queueId-to-timestamp branch from f7ce5b2 to b62a297 Compare December 10, 2023 17:40
@PatrickKa PatrickKa merged commit 5a4e625 into master Dec 10, 2023
6 checks passed
@PatrickKa PatrickKa deleted the queueId-to-timestamp branch December 10, 2023 17:47
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.

Update COBC-EDU protocol
3 participants