ros-drivers / rosserial

A ROS client library for small, embedded devices, such as Arduino. See: http://wiki.ros.org/rosserial
517 stars 525 forks source link

Noetic release #499

Closed doisyg closed 4 years ago

doisyg commented 4 years ago

Hi, Is there something blocking it?

mikepurvis commented 4 years ago

Both rosserial_python tests are currently disabled:

https://github.com/ros-drivers/rosserial/blob/04ddab73643e617910cbfdfe6dcf60476385bffe/rosserial_test/CMakeLists.txt#L47-L49

I believe it can be got going again with the rosserial_python changes extracted from #480 (that PR had become a bit of an omnibus):

https://github.com/ros-drivers/rosserial/pull/480/files#diff-2d3c0dc20802b1a68f2ff2955a5037ed

I (or someone) needs to get back to it so our initial Noetic version isn't completely broken. :)

theseankelly commented 4 years ago

I submitted PR #500 yesterday based on some weekend hacking. Totally missed this issue and the aforementioned PR #480

I'm not a python expert but my PR seems to get the trick done for rosserial on TurtleBot3 for OpenCR.

theseankelly commented 4 years ago

I ended up closing my PR (#500). My changes missed a few things which are already in #480. Plus, #480 has already gone through code review. I think the best path is to rebase and merge #480.

BriceRenaudeau commented 4 years ago

There is still Python compatibility errors in PR (#500 and #480) : in class ServiceServer: in def callback(self, req): Line 171(in PR 500)

if self.parent.send(self.id, data_buffer.getvalue()) >= 0:
            while self.response is None:
                pass

This makes fatal crashes when calling service on the device. -> NoneType can not be compared with Int!

Need to be replaced by :

self.parent.send(self.id, data_buffer.getvalue())
        while self.response is None:
            pass

This change was tested on real robot.

Do I need to make a PR with all python 3 compatibilities like @theseankelly (thanks for pre-clean) or wait for the modification to be included in PR #480?

asukiaaa commented 4 years ago

I could run rosserial on noetic with using gcc7-fixes branch that was created for PR https://github.com/ros-drivers/rosserial/pull/480.

@mikepurvis Thank you for updating codes. When will the PR https://github.com/ros-drivers/rosserial/pull/480 be merged?

mikepurvis commented 4 years ago

@asukiaaa The main gap is that #480 ended up being a jumble of unrelated changes as well. If you (or anyone else) is willing to extract just the Python 3 porting stuff from it and put up a new PR with those changes, I will review and merge it promptly.

asukiaaa commented 4 years ago

@mikepurvis Thank you for information. I created a PR https://github.com/ros-drivers/rosserial/pull/508.

How about to close PR https://github.com/ros-drivers/rosserial/pull/480 if you don't plan to maintain it? PR https://github.com/ros-drivers/rosserial/pull/500 is closed because the PR https://github.com/ros-drivers/rosserial/pull/480 is opened.

mikepurvis commented 4 years ago

@stertingen @doisyg @theseankelly @BriceRenaudeau @asukiaaa @ben-green-kerro

Thanks everyone for your interest in Noetic rosserial. I would like to cut the release in the next day or two; please give your use cases a try with the current source and confirm that all seems sane.

ben-green-kerro commented 4 years ago

Just given it a go, works perfectly for me. Thanks so much for your work on this.

Got a couple of questions just for my own learning. What does 'cut the release' mean? I don't know the lingo. Also, what was needing fixing with it? Just out of curiosity because I had a play with the source but didn't get anywhere.

Thanks!

asukiaaa commented 4 years ago

Thank you for preparing for releasing. I use rosserial with using rosserial-arduino. I could show list of topic and echo them with using nodetic-devel branch.

It seems no problem in my case.

mikepurvis commented 4 years ago

@ben-green-kerro The ROS release process is centered around a tool called Bloom, which uses the package.xml dependency metadata to prepare OS-specific release artifacts for Ubuntu, Debian, Fedora, and others. Creating releases allows users to sudo apt install ros-noetic-rosserial-xyz.

See http://wiki.ros.org/bloom for more details on this.

mikepurvis commented 4 years ago

Release is pending: https://github.com/ros/rosdistro/pull/26310

BriceRenaudeau commented 4 years ago

Hi, I'm sorry to reopen the Issue.

The trouble I got is still here. The last release didn't include the modification I proposed (1 June) L171 must be replaced by: self.parent.send(self.id, data_buffer.getvalue())

Hence I got the same error even with the new release.

[ERROR] [/teensy_leds_serial_node]: Error processing request: '>=' not supported between instances of 'NoneType' and 'int'
['Traceback (most recent call last):\n', '  File "/opt/ros/noetic/lib/python3/dist-packages/rospy/impl/tcpros_service.py", line 632, in _handle_request\n    response = convert_return_to_response(self.handler(request), self.response_class)\n', '  File "/opt/ros/noetic/lib/python3/dist-packages/rosserial_python/SerialClient.py", line 171, in callback\n    if self.parent.send(self.id, data_buffer.getvalue()) >= 0:\n', "TypeError: '>=' not supported between instances of 'NoneType' and 'int'\n"]
mikepurvis commented 4 years ago

@BriceRenaudeau Please send a PR for this change, and I'll merge in time for the next Noetic release (which will be soon, owing to needing to release the fix for the rosserial_server build break.

mikepurvis commented 4 years ago

Both fixes should be in 0.9.1: https://github.com/ros/rosdistro/pull/26506

masynthetic commented 3 years ago

So what is the best way to install currently? I am getting unable to locate on

sudo apt-get install ros-noetic-rosserial

and

sudo apt install ros-noetic-rosserial-xyz

with or without -xyz. Does it need to be built from source?

jspricke commented 3 years ago

@masynthetic sudo apt-get install ros-noetic-rosserial works fine for me on Ubuntu 20. Please check that your apt cache is up to date by doing sudo apt update. Also, this issue is closed and please ask user questions on https://answers.ros.org/