QVPR / teach-repeat

Visual teach and repeat navigation for ground based mobile robots. Code supporting the IROS2021 paper "Fast and Robust Bio-inspired Teach and Repeat Navigation"
BSD 2-Clause "Simplified" License
43 stars 10 forks source link

Creating files for MiRo-B robot platform capability #38

Closed n10200568 closed 2 years ago

n10200568 commented 2 years ago

I have created new files for miro-b robot, which should not have impact on the original code. To be aware, the image_processing_miro_b.py does not use the exact same code as the image_processing.py due to some array issues, this will be investigated further. The creation of new teleop_joy is due the reason that MiRo-B does not use TwistStamp, it simply uses Twist message type.

Dominic-DallOsto commented 2 years ago

For future reference, you can push more commits to this branch to update your pull request, you don't need to make a new one each time

n10200568 commented 2 years ago

From memory, the image_processing says something along different shape between the two arrays. So I went back to the one you committed few months ago that allows other robots to do the stitching and processing (commit f6b5eb51596581f6db9b008eddb4ba8c61fd7f78).

Dominic-DallOsto commented 2 years ago

That version of image_processing should basically be the same as the current version I added the other day - I just added a check to make sure the patch_size is odd in the latest version, which shouldn't affect the stitching/blending.

n10200568 commented 2 years ago

My bad I have realized that I have commented out the rectify_stitch_stereo_image function call within the image_stitcher_miro_b. So effectively it’s just setting the calibration files to None. But the following error is the one I get if running the recent code.

line 478, in rectify_stitch_stereo_image stitched[:,:warped_left.shape[1]] = warped_left * blend_map_linear ValueError: operands could not be broadcast together with shapes (240,520) (600,)

Another question, how did you generate your calibration files, is it simply scaling? (camera resolution of Miro-B is 320x240). Or did you run the camera_calibration.launch to generate the data as well as the image you have in https://github.com/QVPR/teach-repeat/issues/36. I have tried to create the files but not sure what the configuration data is.

Thank you

Dominic-DallOsto commented 2 years ago

Ahh, I think I see the problem that I never added a way for the parameters to be passed through - I'll fix that.

But yeah, can you use camera_calibration.launch to get the calibration files for each camera. There are some good ROS tutorials for this here and here if you've never done it before. Actually we just want a calibration file for each camera individually, we don't need the stereo calibration.

Dominic-DallOsto commented 2 years ago

Ok, I made some changes that should help. Can you pull from here https://github.com/QVPR/teach-repeat/tree/pr38 into this branch? Alternatively, on the right hand side on github there should be an option to allow me permissions to push to your fork - I think this isn't enabled at the moment because I'm getting an authentication error?

You might need to modify the extra_pixels_proportion and blank_pixels_proportion parameters in the launch files to make the stitching work nicely, but it should at least run I think.

n10200568 commented 2 years ago

The permission is already enabled, so not sure why it is not working, I have created a new branch in my repository from the branch you have mentioned.

I will definitely take a look into the materials you have provided and I will give it another go once I have the chance to do so. Thank you for you help Dominic.

Dominic-DallOsto commented 2 years ago

Hmm, that's weird. Maybe it's me doing something wrong then haha. When you get the chance it should work to merge the pr38 branch into your master and the PR will update.

No worries though!

Tobias-Fischer commented 2 years ago

Hi @Dominic-DallOsto @n10200568 - what's the status of this PR?

n10200568 commented 2 years ago

I have merged the pr38 branch (mentioned by @Dominic-DallOsto) into my master branch, and everything works without conflicts. From memory I have removed the image_processing_miro_b.py file on my local files, but have not yet pushed through to the repository. I will double check later today and make the necessary changes, thank you.

n10200568 commented 2 years ago

I have pushed the required files on to my repository. Everything should be fine to merge without affecting the original codes. The modification to files data_collect.py, image_matcher.py and image_sticher.py was from the merge of pr38 branch into master branch on my repository.

Dominic-DallOsto commented 2 years ago

I think this looks OK code-wise.

The configuration for the image stitching should be verified before merging though

Then I think we should be all good

Tobias-Fischer commented 2 years ago

Hi @n10200568 - I suggest to keep this PR limited to what is written in the title (i.e. compatibility with MiRo-B instead of MiRo-E), and open a separate PR that contains the multi-robot stuff. Otherwise it's too hard to review. Especially considering that we were "nearly there" with just the changes needed for MiRo-B.

n10200568 commented 2 years ago

Camera calibrations had been updated, where previous commits of the multi-robot system have been reverted, so only relevant files are present for the miro-b compatibility.

Tobias-Fischer commented 2 years ago

Ta - @Dominic-DallOsto any chance you could have another look?

Dominic-DallOsto commented 2 years ago

Cool, that looks good! Can you just change the names of the calibration files to be eg. left_240_miro_b, and change the comments inside to say these calibration files are for the Miro-B and how you got them (I guess doing a calibration at 240*320)?

Then can you send an image of the output of the image stitcher, just to check how it looks?

Thanks!

n10200568 commented 2 years ago

Here are two images with different parameters. 000000 The above one is with empty_pixel=0 and blank_pixel=0.1. 000000 This one is with empty_pixel=0 and blank_pixel=0.5.

@Dominic-DallOsto I am unsure how you did the unwrap image like in #36 and I have tried to tune the variables. But however I change the variables it seems like the images are overlaying on top of each other, rather than only overlap on the edges so it looks like a panorama view. Will it be like FOV parameters or other parameters that can cause this issue?

Dominic-DallOsto commented 2 years ago

It look like there's a rotation between the left and right images which is weird.

Hmmm...

Could you save an image each from the left and right cameras, then run the image_processing.py script locally to do the following:

If you could post those images, that should help us debug to see where the issue is at the moment.

Thanks!

On 5/09/2022 8:17 am, Kai-Cherng Lee wrote:

Here are two images with different parameters. 000000 https://user-images.githubusercontent.com/62236175/188371715-91d90fde-4d6d-4d33-b2de-8877939ab9f1.png The above one is with |empty_pixel=0| and |blank_pixel=0.1|. 000000 https://user-images.githubusercontent.com/62236175/188371621-2afb6f1f-9341-4a29-a0b6-c47fffa7aa39.png This one is with |empty_pixel=0| and |blank_pixel=0.5|.

@Dominic-DallOsto https://github.com/Dominic-DallOsto I am unsure how you did the unwrap image like in #36 https://github.com/QVPR/teach-repeat/issues/36 and I have tried to tune the variables. But however I change the variables it seems like the images are overlaying on top of each other, rather than only overlap on the edges so it looks like a panorama view. Will it be like FOV parameters or other parameters that can cause this issue?

— Reply to this email directly, view it on GitHub https://github.com/QVPR/teach-repeat/pull/38#issuecomment-1236578694, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGM5S3CMD5DNVGQUMJQDCC3V4WF5ZANCNFSM52IT3RZA. You are receiving this because you were mentioned.Message ID: @.***>

n10200568 commented 2 years ago

Below are the two raw images: miro_b_caml Left miro_b_camr Right

When using rectify_stitch_stereo_image() function locally with, extra_pixels=0, and blank_pixels=0. The below is the stitched image: miro_b_both

And when extracting the warped_left and warped_right images, from rectify_stitch_stereo_image(), these are what I have got: miro_b_warped_left Warped Left miro_b_warped_right Warped Right

Dominic-DallOsto commented 2 years ago

OK, sorry about that! I forgot about having the add the rotation divergence (the left camera being rotation 27 degrees to the left and vice versa) to the individual camera calibration files.

Can you try that again now?

n10200568 commented 2 years ago

The change of the rotation divergence worked. Although I am not too sure how you got the rectification matrix. Was it using the stereo camera calibration method? Because @Dominic-DallOsto you previously mentioned we only need to calibrate it individually so I am unsure was that the issue.

But the results are shown here. miro_b_both_fixed Both Image Combined (extra_pixels_ratio=0, blank_pixels_ratio=0) miro_b_warped_left_fixed Left Warped Image miro_b_warped_right_fixed Right Warped Image

Modifying (extra_pixels_ratio=0.5, blank_pixels_ratio=0.36) miro_b_both_updated4

Although there is still a slight mismatch at the center, but I presume that's due to the rectification_matrix in the calibration file. So if the rectification_matrix is acquired by using the stereo camera calibration method, I am happy to do so to get a better result.

Dominic-DallOsto commented 2 years ago

Ahh nice - that looks a lot better!

I got the recitifcation matrix just using the geometry of the camera placement from the docs - each camera is rotated 27 degrees horizontally from the centre line - so the rectification matrix is just a rotation matrix for 27 degrees to rotate both images to be as if taken from facing straight ahead.

Like you say, that's not perfect and actually calibrating the stereo offset would be better. I never managed to do this successfully though because there isn't a lot of stereo overlap between the cameras so I found that the automatic stereo camera calibrator ROS package didn't work.

If you have enough time to try it out / really need the image stitching to be nice, maybe it's worth trying. But overall I think it should be good enough after downscaling the image a bit anyway. You could also just try changing the rectification matrix to be a rotation of a degree or two more or less and see if that improves it.

Happy to merge this now when you're ready. Thanks a lot for your work!

Tobias-Fischer commented 2 years ago

Awesome! I'd say let's merge this now, and then do the remaining changes in the next PR with the multi robot stuff.