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

Add the ability to prioritize sending messages #276

Open
bakul14 opened this issue Jul 3, 2024 · 5 comments
Open

Add the ability to prioritize sending messages #276

bakul14 opened this issue Jul 3, 2024 · 5 comments
Labels
type: enhancement PR to improve the project.

Comments

@bakul14
Copy link

bakul14 commented Jul 3, 2024

⚡ Feature Request

It is necessary to implement setting the priority subject and service messages, since this feature is available in libcanard.
Here is a piece of implementation of the publish(T const & msg) method. Priority is not a parameter and it is set to CanardPriorityNominal.

template<typename T>
bool Publisher<T>::publish(T const & msg)
{
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpedantic"
  CanardTransferMetadata const transfer_metadata =
  {
   .priority       = CanardPriorityNominal,
    .transfer_kind  = CanardTransferKindMessage,
    .port_id        = _port_id,
    .remote_node_id = CANARD_NODE_ID_UNSET,
    .transfer_id    = _transfer_id++,
  };
#pragma GCC diagnostic pop

There are 2 possible solutions:

  1. Add the ability to set priority through the publisher/subscriber/client/server constructor.
  2. Pass priority as a parameter to the publish() method and others.
@bakul14 bakul14 added the type: enhancement PR to improve the project. label Jul 3, 2024
@aentinger
Copy link
Member

I am willing to merge a high quality pull request providing this functionality using the first approach.

@bakul14
Copy link
Author

bakul14 commented Jul 3, 2024

Okay, it's good. I am ready to check your PR

@aentinger
Copy link
Member

I am willing to receive your PR ;)

@bakul14 bakul14 changed the title [Feature Request] Add the ability to prioritize sending messages Jul 3, 2024
@bakul14
Copy link
Author

bakul14 commented Jul 3, 2024

Ive performed a local commit and am ready to show it, I need write rights to this repository

@pavel-kirienko
Copy link
Member

@bakul14 that's not how it works. You need to fork this repository, push your changes there, then send a pull request: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

image

bakul14 added a commit to bakul14/107-Arduino-Cyphal that referenced this issue Jul 3, 2024
…ng publishers, clients, server through their constructors with the default CanardPriorityNominal priority param
bakul14 pushed a commit to bakul14/107-Arduino-Cyphal that referenced this issue Aug 4, 2024
bakul14 pushed a commit to bakul14/107-Arduino-Cyphal that referenced this issue Aug 4, 2024
…essage transmit priority have been added
bakul14 pushed a commit to bakul14/107-Arduino-Cyphal that referenced this issue Aug 4, 2024
…essage transmit priority have been added
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement PR to improve the project.
Projects
None yet
Development

No branches or pull requests

3 participants