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

Allow multiple messages to be scheduled with the same ID if the interval is set to 0 #30

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

LandryNorris
Copy link
Contributor

@LandryNorris LandryNorris commented Sep 11, 2024

Our current contract for the interval is as follows:

  • -1 -> Stop a repeated frame. Do not send the last data frame
  • 0 -> Send this data once. Cancel any repeating frames with the same ID.
  • > 0 -> Send data on repeat with a period in milliseconds. If called again with period > 0 and new data, update the data in the periodic message

A caller that provides 0 multiple times can reasonably assume that every message is sent, but this was not the case previously. Only one message could be scheduled at a time with a given ID.

m_sendQueue.push_back(detail::CANThreadSendQueueElement(msg, timeIntervalMs));
} else {
// We don't want to replace elements with zero as the interval. Those should be guaranteed to be sent
detail::CANThreadSendQueueElement* existing = findFirstMatchingIdWithNonZeroInterval(msg.GetMessageId());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After this change, calling with (data1, 0), then immediately calling with (data2, 5) will ensure that data1 is always sent once, then data2 is sent every 5 milliseconds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, more practically, calling with (data_i, 0) repeatedly will ensure all the data actually gets sent.

@ItsHarper
Copy link
Contributor

Nice find!

@LandryNorris
Copy link
Contributor Author

@@ -84,20 +84,41 @@ class DriverDeviceThread {
return nullptr; // If no matching element found
}

detail::CANThreadSendQueueElement* findFirstMatchingIdWithNonZeroInterval(int targetId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like findFirstMatchingId() can be deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted

@LandryNorris LandryNorris merged commit 45b44a6 into main Sep 11, 2024
1 check passed
@LandryNorris LandryNorris deleted the fix/repeat-messages-with-0-interval branch September 11, 2024 16:40
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.

2 participants