async Service handler fails due to bad result type

[svanderweyst-cpr]

Description

• Library Version: 1.3.0
• ROS Version:
• Platform / OS: Ubuntu

Steps To Reproduce

const service = new ROSLIB.Service({
  ros,
  name: '/foo/testing',
  serviceType: 'std_srvs/SetBool'
});

service.advertise(async (req, resp) => {
  console.log('req:', req);
  resp.success = req.data;
  resp.message = `passed ${req.data}`;
  return true;
});

Expected Behavior
rosservice call /foo/testing "data: false"
should output
success: False
message: "passed false"

Actual Behavior
rosservice call hangs
rosbridge reports:
service_response: Expected
\ field result to be one of (<class 'bool'>,). Invalid value: {}"
file: "protocol.py"

in Service.js, changing _service response to start like this seems to resolve the issue in my small use case:

Service.prototype._serviceResponse = async function(rosbridgeRequest) {
  var response = {};
  var success = await this._serviceCallback(rosbridgeRequest.args, response);

[MatthijsBurgh]

I have really no clue what will happen to non async service callbacks. So a good way to check whether this works, add a test with an async and non-async service callback. That non-async test should succeed and the the async test should fail. Then add your changes and both tests should succeed.

[f-o-w-l]

@MatthijsBurgh Why was fixing this issue not planned? Why doesn't it make sense to use an async callback? The person that opened this issue suggested a fix that would work and you said "Then add your changes and both tests should succeed" - so why not implement those changes?

If these changes were implemented and a synchronous callback function was provided, wouldn't it still work correctly exactly the same way if the _serviceResponse function is typed as async and the await for this._serviceCallback is added?

[MatthijsBurgh]

not planned is also used for stale issues. This was issue was stale. I didn't have the time to implement it back then. And I probably missed the suggestion, when I did a quick cleanup of the stale issues in 2023.

@f-o-w-l Your help would be really appreciated. So please implement a test, which fails for the async callback before the fix. Apply the fix and show the test now succeeds.

[f-o-w-l]

@f-o-w-l Your help would be really appreciated. So please implement a test, which fails for the async callback before the fix. Apply the fix and show the test now succeeds.

I would love to contribute! Let me give that a shot some time soon 😁