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

Also require the order ID in the delivery message #1

Merged
merged 2 commits into from
Sep 4, 2018

Conversation

morxa
Copy link
Member

@morxa morxa commented Aug 9, 2018

When delivering a product, the robot now also needs to send the ID of
the order that the product belongs to.

This idea was discussed at TC meeting 2/19.

@morxa morxa requested a review from a team as a code owner August 9, 2018 14:46
When delivering a product, the robot now also needs to send the ID of
the order that the product belongs to.
@morxa morxa force-pushed the order-id-in-ds-prepare-msg branch from 7cf2a66 to 974dd6c Compare August 9, 2018 14:59
Copy link

@thomerz thomerz left a comment

Choose a reason for hiding this comment

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

Shouldn't we change this sentence:

[...] points can only be scored if the DS was properly prepared before (means the corresponding delivery
lane which was desired in the refbox order) [...]

to:

[...] points can only be scored if the DS was properly prepared before (means the corresponding order ID
[and lane which was desired in the refbox order]) [...]

Since we are submitting the order ID, we would not need to specify the lane anymore? Anyhow, we should award points (automatically!) only if the order ID is specified

Make clear that points are only given for delivery if the order ID was
set beforehand.
@morxa
Copy link
Member Author

morxa commented Aug 23, 2018

Thanks for pointing that out. I rephrased that paragraph to make clear that points are only given if the DS was prepared with an order ID.

I haven't removed the lane. From my side, we can keep it or drop it completely from the prepare message. It's not really necessary anymore, but it also doesn't do any damage.

Copy link
Member

@timn timn left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@MostafaGomaa MostafaGomaa requested a review from timn September 4, 2018 08:20
@timn timn merged commit d580e44 into master Sep 4, 2018
@morxa morxa deleted the order-id-in-ds-prepare-msg branch October 19, 2018 17:41
@morxa morxa added the accepted-rule-change The rule change has been accepted label Feb 14, 2019
@morxa morxa added this to the RoboCup 2019 milestone Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted-rule-change The rule change has been accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants