Skip to content

Conversation

mamueluth
Copy link
Member

@mamueluth mamueluth commented Feb 14, 2024

Can be tested with b-robotized-forks/ros2_control_demos#10
TODOS:

  • Set initial values in on_init
  • Add other Transmission types
  • Add tests

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0bdcd41) 47.53% compared to head (5b406aa) 46.38%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
- Coverage   47.53%   46.38%   -1.15%     
==========================================
  Files          41       39       -2     
  Lines        3547     3253     -294     
  Branches     1930     1749     -181     
==========================================
- Hits         1686     1509     -177     
+ Misses        459      458       -1     
+ Partials     1402     1286     -116     
Flag Coverage Δ
unittests 46.38% <ø> (-1.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 5 files with indirect coverage changes

Comment on lines 104 to 122
// used for the Transmission pass through.
// read: actuator_interface.state_->Transmission->joint_interface.state_
// write: joint_interface.command_->Transmission->actuator_interface.command_
// And StateInterface(joint_interface.state_)
struct InterfaceData
{
// TODO(Manuel) set initial to NaN and on_init initialize to given value in info or 0.0
explicit InterfaceData(const std::string & name)
: name_(name),
command_(0.0), // command_(std::numeric_limits<double>::quiet_NaN()),
state_(0.0), // state_(std::numeric_limits<double>::quiet_NaN()),
transmission_passthrough_(
0.0) // transmission_passthrough_(std::numeric_limits<double>::quiet_NaN())
{
}

std::string name_;
double command_;
double state_;
// this is the "sink" that will be part of the transmission Joint/Actuator handles
double transmission_passthrough_;
};
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about this structure. but also it looks like something that might be added to .cpp file. Especially, since we have this game with 0.0 and quiet_NaN().

I have to check the implemenation to tell more about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

what about the struct? need it in the .hpp since we define the lists with it


transmission_interface::JointHandle joint_handle(
joint_info.name, hardware_interface::HW_IF_POSITION,
&joint_interface->transmission_passthrough_);
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 transmission pass_though?

Copy link
Member Author

Choose a reason for hiding this comment

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

have to check this maybe using directly state_, command_ is fine but i think overriding was a problem

Copy link
Member Author

Choose a reason for hiding this comment

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

need it because we would override the handle otherwise (Handle for actuator/joint needs a storage, but we have state and command. We could only give one of the two, so if we call the functions actuator_to_joint or joint_to_actuator we would involunatry override )

std::for_each(
actuator_interfaces_.begin(), actuator_interfaces_.end(),
[](auto & actuator_interface)
{ actuator_interface.transmission_passthrough_ = actuator_interface.state_; });
Copy link
Member

Choose a reason for hiding this comment

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

Ah OK, this is just a storage. Can't we use directly actuator_interface.state_?

Copy link
Member Author

Choose a reason for hiding this comment

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

no see above comment

[](auto & actuator_interface)
{ actuator_interface.command_ = actuator_interface.transmission_passthrough_; });

// simulate motor motion
Copy link
Member

Choose a reason for hiding this comment

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

Is this important for something?

Copy link
Member Author

Choose a reason for hiding this comment

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

sry what do you exactly mean? i dont understand the question

Copy link

mergify bot commented Feb 19, 2024

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

@mamueluth mamueluth force-pushed the mock_hw_transmissions branch from 5b406aa to 687b2a3 Compare February 19, 2024 10:42
@mamueluth mamueluth force-pushed the mock_hw_transmissions branch from 73811dd to e27e79a Compare February 27, 2024 17:48
Copy link

mergify bot commented Apr 29, 2024

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

Copy link

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added the stale label Apr 11, 2025
Copy link

mergify bot commented Apr 11, 2025

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

@github-actions github-actions bot removed the stale label Apr 14, 2025
Copy link

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added the stale label May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants