clubcapra / markhor

🐐 Capra-Markhor is a ROS-based solution for managing and operating Club Capra's second rescue robot. 🐐
9 stars 1 forks source link

added tpv_controller.launch to parkour.launch #42

Closed Leo-Dan closed 2 years ago

Leo-Dan commented 2 years ago

The tpv didnt seem to work when launched via common.launch inside parkour. I removed it form common.launch and added it to parkour.launch. It fixed the problem so Im happy :)

saxtot commented 2 years ago

Why the change?

Leo-Dan commented 2 years ago

Why the change?

Leo-Dan commented 2 years ago

Because it did not work every time, I had to launch it separatly

saxtot commented 2 years ago

Because it did not work every time, I had to launch it separatly

I understand, but you didn't actually change anything to solve the problem from what I can see. Moving the line around shouldn't have an impact. If it does, there is something weird there that we need to understand.

Leo-Dan commented 2 years ago

Because it did not work every time, I had to launch it separatly

I understand, but you didn't actually change anything to solve the problem from what I can see. Moving the line around shouldn't have an impact. If it does, there is something weird there that we need to understand.

it does have an impact

saxtot commented 2 years ago

Are you able to tell me why there is? Because, from my understanding, you are just launching it later.

Leo-Dan commented 2 years ago

Are you able to tell me why there is? Because, from my understanding, you are just launching it later.

Unfortunatly not my man ! But I totally agree with you on the nonsense it represents. Because it should work. I definetly want to understand ROS nodes launch sequence better.

saxtot commented 2 years ago

In that case, we should investigate. We already know our launch files are clashing, so that is what actually needs to be fixed. "It just works, I don't know why" is, in the vast majority of cases, not a good way to proceed. Especially when working with a 80kg robot! We created problems for ourselves often in the past working that way.

Leo-Dan commented 2 years ago

In that case, we should investigate. We already know our launch files are clashing, so that is what actually needs to be fixed. "It just works, I don't know why" is, in the vast majority of cases, not a good way to proceed. Especially when working with a 80kg robot! We created problems for ourselves often in the past working that way.

Totally understandable, what do you suggest I try ? What do you mean by investigate ?

saxtot commented 2 years ago

Investigating would be going back to your basic problem solving methods. The priority is finding the problem's origin. There are multiple ways to work, but I would suggest simplifying your problem to its simplest version and then adding back elements step by step until you can recreate it. Then, you can isolate it and begin working on a solution.

GLDuval commented 2 years ago

Investigating would be going back to your basic problem solving methods. The priority is finding the problem's origin. There are multiple ways to work, but I would suggest simplifying your problem to its simplest version and then adding back elements step by step until you can recreate it. Then, you can isolate it and begin working on a solution.

Could this be a problem with ROS? Maybe the node isn't launched because of ROS and not because of our code

Leo-Dan commented 2 years ago

I think a good solution would be to take the nodes that common launches and bringing them back into parkour. The goal is the fast deployment of the robot. The separation of the launch files was made to patch the same type of bug this PR tries to solve, right ? So I would bring them together, see how it reacts, and go from there with the debugging.

saxtot commented 2 years ago

I think a good solution would be to take the nodes that common launches and bringing them back into parkour. The goal is the fast deployment of the robot. The separation of the launch files was made to patch the same type of bug this PR tries to solve, right ? So I would bring them together, see how it reacts, and go from there with the debugging.

Well, two things:

Could this be a problem with ROS? Maybe the node isn't launched because of ROS and not because of our code

It could be. However, we don't have any proof of it either. Let's get more data on the issue. Then, we'll know if our code has an issue(s) or if we have to bypass the issue somehow.

benmalenfant commented 2 years ago

Could we please have a description of the bug this is supposed to fix?

benmalenfant commented 2 years ago

Also we could take a look at the logs next times the issue happens

Leo-Dan commented 2 years ago

This PR should definitely not be merged, my bad, just wanted to test things out. This thing doesnt need to be reviewed, we need to do a lot more testing. I'm willing to do that. You guys are totally right, esti !

That being said, The problem is : the tpv node does not launch when launched from parkour.launch file, it sometimes works (not often). Seems to be a launch timing issue. Maybe its because it uses Ros serial/ communicating via USB.

Leo-Dan commented 2 years ago

Also we could take a look at the logs next times the issue happens

Always do and will, in the future I'll capture it so guys can see.

benmalenfant commented 2 years ago

Allright, if we find that rosserial_arduino node is the issue here, keep in mind we could use rosserial_python, those two seem to differ a bit, one might be more stable than the other

Leo-Dan commented 2 years ago

Allright, if we find that rosserial_arduino node is the issue here, keep in mind we could use rosserial_python, those two seem to differ a bit, one might be more stable than the other

Cool thanks ! If you have some idea on how to test that thoroughly let me know

saxtot commented 2 years ago

Great plans, I'll close this, update the Teams task and link this PR there. We'll start a fresh PR when we'll have this figured out.