Saving tar fails in python 3.8

[PfeifferMicha]

When pressing "save" in cameracalibrator, the tool fails to save the tarballs and prints:

...
/image_pipeline/camera_calibration/src/camera_calibration/calibrator.py", line 1266, in taradd
    if isinstance(buf, basestring):
NameError: name 'basestring' is not defined

It looks like "basestring" is deprecated in python 3.8. That document recommends replacing basestring with str, however then saving still fails with

Traceback (most recent call last):
  File "/home/user/Projects/ROS/catkin_ws_noetic/src/image_pipeline/camera_calibration/src/camera_calibration/camera_calibrator.py", line 273, in on_mouse
    self.c.do_save()
  File "/home/user/Projects/ROS/catkin_ws_noetic/src/image_pipeline/camera_calibration/src/camera_calibration/calibrator.py", line 578, in do_save
    self.do_tarfile_save(tf) # Must be overridden in subclasses
  File "/home/user/Projects/ROS/catkin_ws_noetic/src/image_pipeline/camera_calibration/src/camera_calibration/calibrator.py", line 1279, in do_tarfile_save
    taradd('left.yaml', self.yaml("/left", self.l))
  File "/home/user/Projects/ROS/catkin_ws_noetic/src/image_pipeline/camera_calibration/src/camera_calibration/calibrator.py", line 1275, in taradd
    tf.addfile(tarinfo=ti, fileobj=s)
  File "/usr/lib/python3.8/tarfile.py", line 1995, in addfile
    copyfileobj(fileobj, self.fileobj, tarinfo.size, bufsize=bufsize)
  File "/usr/lib/python3.8/tarfile.py", line 256, in copyfileobj
    dst.write(buf)
  File "/usr/lib/python3.8/gzip.py", line 276, in write
    data = memoryview(data)
TypeError: memoryview: a bytes-like object is required, not 'str'

I currently do not know enough about tarballs to fix this myself, maybe someone can have a look?

Working on noetic branch, commit 9836401

[SteveMacenski]

@PfeifferMicha thanks for the note. Can you try memoryview(bytes("This is a string", encoding='utf-8'))? That should get you the right types. If that all works out, please submit a PR and we'd be happy to merge it in

[PfeifferMicha]

Done, pull request is open. I ended up removing the dependency on StringIO entirely, as using it would give me a lot of troubles downstream. Saving now works on python3.8, Ubuntu 20.04.

Side Note: Since the current head of branch 'noetic' is broken (Charuco integration is halfway done), I made this pull request on top of the previous commit, 9836401. See also #550.
Maybe the Charuco implementation could be moved to another (temporary) branch until it is fixed? Or is 'noetic' considered a development branch? In this case, is there a "stable" noetic branch?

[SteveMacenski]

What do you mean its half implemented? (@JStech) Please describe.

[JStech]

I don't currently have a Noetic install, so I just cherry-picked the commits and trusted CI. Everything is there. I can probably look into it this weekend.

[SteveMacenski]

I think we need more info from @PfeifferMicha. Functionally noetic and melodic shouldn't really vary much so I want to know what they believe is "halfway done" to know what needs updates.

[PfeifferMicha]

By "halfway done" I mean I needed to change quite a few things to get it to not crash. Then once that was done I noticed Stereo Calibration (my actual goal) was not implemented yet.
I had closed #550 since I thought this was sort of intentional (with "WIP" in the commit message), I'll reopen it and give more details there.

[SteveMacenski]

@JStech can you follow up on #550? Looks like your commit may have introduced some issues.

Stereo for those tags haven't been implemented yet, you're welcome to do so. Its often a pain to implement a new tag for mono/stereo/fisheye so its not expected that someone does all of them (and in some cases the library may not support them)

[JStech]

Yeah, I'll check it out. "Too many values to unpack" is an error I became very familiar with in implementing this feature, so it's probably my fault. Sorry about that.

And yes, stereo and fisheye are both unimplemented with the ChArUco board at this point because OpenCV doesn't have those functions implemented for ChArUco boards, and, frankly, it's not something I need right now.

[SteveMacenski]

if its not in openCV, that's enough of a reason for me

[JWhitleyWork]

@JStech / @PfeifferMicha Can you please verify that #554 fixed this issue?

[PfeifferMicha]

With #550 still open, I cannot verify, as (stereo) calibration fails, so I never get to the point where I can save. As soon as #561 is working and merged, I can try to verify this (for noetic, I still don't have a ros2 setup).

[JWhitleyWork]

@PfeifferMicha Can you please re-test this now that #561 is merged?

[mikeferguson]

Presumed to be fixed by last noted commit, since no further feedback