search

RobotWebTools / roslibjs

The Standard ROS JavaScript Library
https://robotwebtools.github.io/roslibjs
Other
659 stars 372 forks source link

Add `advertiseAsync` method to `Service` #698

Closed EzraBrooks closed 3 months ago

EzraBrooks commented 3 months ago

Public API Changes

Adds an advertiseAsync function to the Service class to allow Promise-returning functions as callbacks to services.

Description

☝🏻 This is useful for cases where, like a feature I'm working on right now in my application, you want to wait to return a Service call until a different asynchronous operation completes. The original advertise function is very limiting because it requires all operations undertaken by the Service server to be synchronous due to the design of asynchronicity in JavaScript disincentivizing synchronizing async closures into synchronous ones.

EzraBrooks commented 3 months ago

I believe the existing unadvertise should work here - note that the existing unadvertise doesn't actually remove the event handler from the EventEmitter, so no changes are needed to make this behave the same. That's actually probably a bug - maybe worth fixing for both the synchronous and asynchronous version at the same time.

MatthijsBurgh commented 3 months ago

That's actually probably a bug - maybe worth fixing for both the synchronous and asynchronous version at the same time.

@EzraBrooks sounds like a good idea

EzraBrooks commented 3 months ago

I'll merge this and open up a follow-up PR that fixes unadvertise for both cases (and add a relevant unit test).