strands-project / strands_perception_people

long-term detection, tracking and recognition of people
96 stars 70 forks source link

Fixing error in https://github.com/strands-project/strands_perception_people/issues/163 #165

Closed ferdianjovan closed 9 years ago

ferdianjovan commented 9 years ago

Fixing error length problem in get_trajectory_message function, making visualisation adaptive to the length of poses (preventing error),

Add the option to choose trajectories from a specific map.

ferdianjovan commented 9 years ago

Tested in simulation.

hawesie commented 9 years ago

Is this needed at G4S?

ferdianjovan commented 9 years ago

Actually, we can use the current one. At the moment, the trajectory stitching will respawn if it crashes. This fixing just prevents the crashes from happening.

PDuckworth commented 9 years ago

@hawesie I will test this. I'll pull it onto Lucie to test the QSRViz code...

PDuckworth commented 9 years ago

@ferdianjovan We tested. It doesn't break anything. But it seems like lots of static objects are being published as trajectories, and if you move some distance, it loses you. Any theories?

ferdianjovan commented 9 years ago

static objects are being published is because they are detected by upper body or leg detector and bayes people tracker treat information from those two detector as a person being detected.

It is out of human_trajectory package as this package only subscribes from bayes_people_tracker topic.

I think the tracker is what the best we have now. You can play with the parameter in https://github.com/strands-project/strands_perception_people/blob/indigo-devel/bayes_people_tracker/config/detectors.yaml to get better results.

ferdianjovan commented 9 years ago

I also added an update for vision_people_logging here as well. The upper body position in world map frame stored by vision_people_logging was wrong due to the wrong transformation from head_xtion_depth_optica_frame to map.

cdondrup commented 9 years ago

I guess @lucasb-eyer should have a look at the changes in the ubd logger.

PDuckworth commented 9 years ago

So to get any sort of filtering on the trajectories in real time, we have to also subscribe to /human_movement_detection_server/detections?

ferdianjovan commented 9 years ago

Yup that is the idea. So we will not lose any potential trajectory that might have been classified as non-human trajectories.

lucasb-eyer commented 9 years ago

@ferdianjovan do you mind opening a separate PR for the ubd_logging stuff to avoid mixing 2 discussions and fixes? You can just git checkout -b fix-ubd-logging to get a new branch and then git rebase -i indigo-devel and remove the two unrelated commit lines, this way you get another branch with only the ubd commit. If that doesn't work for you, then nevermind.

ferdianjovan commented 9 years ago

Ah, ok. I will do that. Sorry I did not know how to do that.

cdondrup commented 9 years ago

You can also use git cherry-pick (my favourite tool).

git checkout indigo-devel
git checkout -b ubd_fix
git cherry-pick decbc5566227adbc334df91046991d23df4bf643
ferdianjovan commented 9 years ago

Got it, opened a new PR https://github.com/strands-project/strands_perception_people/pull/166

lucasb-eyer commented 9 years ago

Ah right, you can delete the commit here in a similar way, e.g. when you're on this branch do git rebase -i indigo-devel and remove the lines of the commits you want to delete. Then you need to "force push" though (only ever do this for PRs): git push -f with also the usual options you pass to git push, if any.

ferdianjovan commented 9 years ago

I think we are ok here since the vision_logging package file is not here anymore.

ferdianjovan commented 9 years ago

Since the review is quite near, could someone check, test and eventually merge this before the review to prevent any error during the review? @PDuckworth have you tested this?

cdondrup commented 9 years ago

I can merge this but I am not able to test this properly as I don't have a clue what this is supposed to do. If @PDuckworth says it's alright, I'll merge.

PDuckworth commented 9 years ago

@cdondrup Pretty sure this can be merged. I've been using it for a while and haven't seen the error again.

cdondrup commented 9 years ago

Thanks @PDuckworth

cdondrup commented 9 years ago

@ferdianjovan want this released?

ferdianjovan commented 9 years ago

yes please.