-
Notifications
You must be signed in to change notification settings - Fork 0
Fix license dynamic payload analysis core #6
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
Conversation
|
@enrico391, it's best to keep pull requests focused on a single purpose. Since this one addresses a license fix, you should avoid including unrelated changes. In the future, try to avoid mixing different features or modifications in the same PR to keep the review process straightforward and efficient. |
|
So, it's better to create a branch with only the files related to the issue and create a PR from it, right ? |
|
yes, just like that |
| self.robot = None | ||
|
|
||
| # frame where the external force is applied | ||
| self.frame_id = "arm_left_7_link" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this frame hardcoded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remove it in the last version (in the last PR)
| def robot_description_callback(self, msg): | ||
| self.get_logger().info('Received robot description') | ||
|
|
||
| self.robot = TorqueCalculator(robot_description = msg.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming coherence is not present here. we shall talk about this in next meeting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok yes, it is better to change that name
| # force = Marker() | ||
| # force.header.frame_id = frame_id | ||
| # force.header.stamp = self.get_clock().now().to_msg() | ||
| # force.ns = "external_force" | ||
| # force.id = id_force | ||
| # force.type = Marker.ARROW | ||
| # force.action = Marker.ADD | ||
| # force.scale.x = 0.20 | ||
| # force.scale.y = 0.05 | ||
| # force.scale.z = 0.05 | ||
| # force.color.a = 1.0 # Alpha | ||
| # force.color.r = 0.0 # Red | ||
| # force.color.g = 0.0 | ||
| # force.color.b = 1.0 # Blue | ||
| # # Set the direction of the arrow based on the external force | ||
| # force.pose.orientation.x = external_force[id_force].linear[0] | ||
| # force.pose.orientation.y = external_force[id_force].linear[1] | ||
| # force.pose.orientation.z = external_force[id_force].linear[2] | ||
| # force.pose.orientation.w = 0.0 # Set to 1.0 for a straight arrow | ||
| # self.publisher_force.publish(force) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I removed it in the last PR
| ], | ||
| install_requires=['setuptools'], | ||
| zip_safe=True, | ||
| maintainer='morolinux', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| maintainer='morolinux', | |
| maintainer='Enrico Moro', |
Fix #4
Add license in core.py and package.xml