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

[WIP] Distributed control poc #1

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

mamueluth
Copy link
Member

No description provided.

@mamueluth mamueluth requested a review from destogl December 2, 2022 14:47
Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

I have to check from where the node comes from and how you add those things into the correct callback group.

controller_manager_msgs::msg::PublisherDescription create_publisher_description_msg() const;

private:
void publish_value_on_timer();
Copy link
Member

Choose a reason for hiding this comment

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

We should publish on events an not on timer. For example when one of read, write or update methods is called.

namespace distributed_control
{

class SubControllerManagerWrapper final
Copy link
Member

Choose a reason for hiding this comment

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

What is this wrapping arround?


virtual std::string get_underscore_separated_name() const override
{
// remove first "/" from namespace and replace all follow occurrences with "_"
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this?

Copy link
Member

@destogl destogl Dec 6, 2022

Choose a reason for hiding this comment

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

Probably needed for node names. Why not use one node name per CM/RM and namespaces, e.g. /<controller_manager>/<hardware>/<joint>/<interface>

return msg;
}

void CommandForwarder::subscribe_to_command_publisher(const std::string & topic_name)
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between “command forwarder” and “command publisher”?

std::flush(std::cout);

// Put the message into a queue to be processed by the middleware.
// This call is non-blocking.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be blocking call?

msg->data = std::numeric_limits<double>::quiet_NaN();
}
RCLCPP_INFO(node_->get_logger(), "Publishing: '%.7lf'", msg->data);
std::flush(std::cout);
Copy link
Member

Choose a reason for hiding this comment

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

Probably not needed?

Suggested change
std::flush(std::cout);

@@ -540,6 +663,58 @@ class ResourceStorage
import_command_interfaces(systems_.back());
}

void add_sub_controller_manager(
Copy link
Member

Choose a reason for hiding this comment

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

Does resource manager have to know about controller manager? It's a bit convoluted.

* fix typo in StatePublisher (used the pointer which has been moved...)
* not a component anymore
* collect StatePublishers in map
* export and imprt statepublisher description
* initialize DistributedHandle through description
* automated generation of node and topic names
* Its not working, since the storage type of command interfaces needs
to be changed to std::shared_ptr. This is done in follow_up since should
change a lot.
@mamueluth mamueluth force-pushed the distributed_control_poc branch from d3feeaa to 71f4757 Compare January 11, 2023 08:29
@mergify
Copy link

mergify bot commented Mar 30, 2023

This pull request is in conflict. Could you fix it @mamueluth?

1 similar comment
Copy link

mergify bot commented Nov 15, 2023

This pull request is in conflict. Could you fix it @mamueluth?

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