API review

Proposer: Daniel Stonier

Present at review:

• Piyush Khandelwal
• Jihoon Lee

Meeting agenda

A small review of things I think we need to overhaul, now we’ve seen some things working. The ros api got us started, but a few things have come up now. Just add comments to interspersed with your name in []'s below.

Links

• Terminology

• Ros Api Initial Discussion

• Current Gateway Development Walkthrough

Ros Api

General Information

• /gateway_info : details about the local gateway

  • request: -

  • response:

    • name, health

    • hub whitelists & blacklists

    • exposed interfaces:

      • public interface & filters

      • flipped interfaces to remote gateways (flipped out)
    • foreign interfaces:
      • flipped interfaces from other gateways (flipped in)
      • pulled interfaces from other gateways

• /remote_gateway_info: list of all gateways connected to the hub along with some info

  • request: - [optional list of gateway names to filter the returned list, empty returns all]

  • response: gateways, public interfaces, blocking status, health

Public Interfaces

• /list_public_interfaces: list all remote public interfaces

  • request: - [optionally list of gateway names to filter the returned list]

  • response: list the public interface for each remote gateway

• /advertise: appends to watchlist, later adds to public interface

  • request:

    • watchlist - heterogenous list of type+names/type+names+node names/type+regex
    • cancel flag - remove instead of append

  • response: updated public interface & watchlist

• /advertise_all: sets watchlist to '*' creates blacklist, later adds to public interface

  • request:

    • blacklist - heterogenous list of type+names/type+names+node names/type+regex

  • response: updated public interface & blacklist

Flipped Interfaces

• /flip : for each gateway, append to watchlist, later flips

  • request:

    • target gateway
    • target name (default '/gateway_name/name')
    • watchlist - type+name or type+name+node name
    • cancel flag - remove instead of append

  • response: updated flipped interface & filter

• /flip_pattern : for each gateway, append to watchlist, later flips

  • request:

    • target gateway
    • target namespace (default '/gateway_name')
    • watchlist - type+regex
    • cancel flag - remove instead of append

  • response: updated flipped interface & filter

• /flip_all : for each gateway, sets watchlist to '*' creates blacklist, later flips

  • request:

    • list of target gateways - optional (all if empty)
    • list of target namespaces - root connections under these (default '/gateway_name')
    • blacklist - heterogenous list of names/names+node names/regex patterns

  • response: updated flipped interface & blacklist

Pulled Interfaces

Probably don't want to do a pull from all gateways at once, you'd need to be careful about namespace collisions.

• /pull

  • request:

    • gateway
    • heterogenous list of type+names/type+names+node names/type+regex patterns
    • namespace - root connections under this (default '/gateway_name')
    • cancel flag

  • response: updated pulled interface

• /pull_all

  • request:

    • gateway
    • namespace - root connections under this (default '/gateway_name')
    • cancel flag

  • response: updated pulled interface

Parameters

• ~hub_uri : static ip/port or hostname/port for hub if not using zeroconf
• ~hub_whitelist : hubs the will look for first (will ignore those not in whitelist)
• ~hub_blacklist : hubs the gateway won't connect to
• ~default_public_interface
  • Connections that should be watched and put into the public interface
• ~default_flipped_interface
  • Connections that should be watched and put into the flipped interface
• Various Filters [watchlists/blacklists]
  • Connections that are put in/banned from the public and flipped interfaces

Comments

• /list_flipped_in_interface, /list_pulled_interface, /list_local_public_interface are all repeated in /gateway_info
  • could conceivably drop these, but no real need.

    • (Piyush) I am typically in the favor of a more concise API. I would prefer dropping these if /gateway_info has the information already

    • (Daniel) I'm fine with that - can simplify list_remote_public_interfaces -> list_public_interfaces, list_flipped_out_interface->list_flipped_interface since there is no need to distinguish between local and remote.

• /flip_all: is not represented above, but coveredy by /flip *

  • (Piyush) Just to clarify, by /flip *, do you mean setting a mode field to ALL. I would prefer having an explicit flag in the message than letting the user supply a ".*" regex filter. If we want to implement PUBLIC, then we'll need a separate flag anyway.

  • (Daniel) how about we implement /flip_all (I don't think that confuses the api at all and its super convenient) and we leave off a /flip_public till one of us needs it?

• /flip_public: is not represented above - either a special wildcard or a separate service..do any of us have a use case for this one yet? Could usually be met by a pull all from the other end.

  • (Piyush) I don't intend to use flip, atleast not as first. However I can see a user specifying default public actions/services/topics as a parameter and then using flip_public more than flip_all. Atleast, that's how I would use it.

  • (Daniel) Flipping is intended to be used for when you want to control who gets your interface, that and freely sharing are kind of mutually exclusive concepts. If you want to flip public, I think it's just as easy to set up the same filters for flipping and control them without publicly sharing them.

    • (Piyush) +1; this is an excellent point.

• /pull_all: is not represented above, but covered by /pull *
• Merging topics and services.

  • (Jihoon) I just want to bring this up from github issue. I think we should keep topics and services separate. We plan to add subscriber in the future for action and it may create a lot of headache to keep pub and sub in the same place. I think there is reason why masterapi manage them separate.

  • (Daniel) I'm starting to also think we can't automatically distinguish. Thinking about subscribers suddenly makes it difficult to distinguish and adding 'subs' only flag seems unintuitive and hackish. Then there's action server vs action client.

  • (Daniel) How about we still merge ros api (e.g. 'advertise', 'flip', 'pull'), but provide a request argument that the user has to provide for the type - pub, sub, service, action client, action server?

  • (Jihoon) +1 on Daniel's suggestion. We can define rosapi like advertise <type:pub,sub,srv,ac,as> <required parameters> ...

  • (Jihoon) So do we use 'advertise' and 'pull' for public interfaces and 'flip' for private synchronization? Um can I suggest use 'subscribe' instead of 'pull'?

    • (Daniel) subscribe starts to get confusing I think....e.g. I want to subscribe that publisher,

  • (Jihoon) FYI, action client has two publisher(goal, and cancel), and three subscribers(feedback, status, and result). So action server should have three publishers and two subscribers.

  • (Piyush) This sounds fine. I would recommend defining a Connection message (string type, string[] info). We can pass a list of these Connection messages to the hub/master instead of the list of tuples that we are passing around right now. The "type" can be an enumeration inside the message, and we'll have to explicitly define how the info tuples should look like for each type.

• (Daniel) Flipping and pulling need namespace targets

Subscribers

(Daniel) Just had a brain freeze and realised we are currently not doing any advertising, flipping or pulling of subscribers, only publishers. Does this change any of the above?

• (Piyush) My use-case does not require this functionality. I'll only have a simple push-pull of publishers. Can you specify a use-case where subscribers will be required.

  • (Daniel) we will have a system where robots will be clients to a centralised super master (~ building manager). These clients in this design are intended to be secure - no open master and no flipping of topics to them from elsewhere. Instead, the clients flip their own interfaces to a shared workspace in the super master (the peer to peer port connections will still be direct, they will just be visible in this centralised shared workspace). This makes debugging, co-ordination, introspection much easier than having pieces shared hither and thither, but it does mean we need both pubs and subs in that centralised workspace.

• (Piyush) The triples/4-tuples will be largely unaffected. We'll have to discuss how subscriber flipping behavior with filters. For instance, you might want to push a topic out to the public API, but you are subscribing to it locally as well. If that topic is in the filter, do you push out the publisher and subscriber both?

  • (Daniel) yeah, that's why I was thinking about providing that hint in my comments below.

Triples

• flips should be continuously watched for in both named and regex cases, impossible for triples
• I think we can probably do away with triples too - these are really specific and can’t be watched for (no idea of the port if we call for it in advanced). Problem is a simple topic name is not specific enough to watch and flip a single publisher sometimes, but a topic and node name is sufficient. From this we can not only determine the node uri when the topic becomes published, but we can put this on a watcher list as well.

  • (Piyush) +1; Supplying the port is going to be tricky. I really like the idea about using topic/node and filter/node combinations. Note: We'll still have to use triples/4-tuples internally.

  • (Daniel) : thinking about subscribers specifically, we may need to optionally supply a hint (pub only or sub only) with any advertise or flip call to narrow down selections (e.g. don't want to flip all pubs and subs of an image topic inside a nodelet manager, just the publisher). Although I'd leave this off the todo list for now until we think we have corner cases for which our rough 'named' or 'named + node name' specification isn't good enough.

    • (Daniel) : I'm cancelling the idea of hints - hackish. Jihoon convinced me of putting connection type (pub/sub/service/action) into the api.

• Above is a big overhaul, but necessary I think

  • (Piyush) +1

Code Cleanup

Currently the code is getting a bit tangled. Redis everywhere for example. Also no really clear reason we need rocon_gateway_sync and rocon_gateway (my fault).

Simple class sketch proposal (I know there are much fancier ways to design a program, but this case is pretty simple):

          Gateway
             |
      --------------
     |              |
Master Api       Hub Api

Gateway

• ros param configuration
• ros api to local system (and callbacks)
• hub connection handler

Master Api

• rosmaster-slave api implementation
• abstract api for master synchronisation

Hub Api

• redis implementation
• abstract api for hub interactions
  • get redis info, formatted for gateway
  • set redis info from gateway formatted input
  • redis pubsub messaging
  • redis pubsub callbacks

Also zeroconf_comms needs to be pulled and made into a unary stack so any dependency doesn't bring in the whole kaboodle.

• (Piyush) The design looks good to me. I would also recommend doing away with rocon_gateway_helper until we have fleshed the code a bit more. At this time, that package is defunct, and the code is duplicated in rocon_gateway_sync.

• (Daniel) Ok, let's go - class refactoring, redundant package merging and zeroconf_comms unary stack.

• (Jihoon) Design looks good to me too. We just need to be careful about local and redis server info mirroring.

Conclusion

Stack status change mark change manifest)

• Class restructuring as above

• Package merging as above

• Drop triples from a user's perspective, use type+name/type+name+node_name

• Target namespaces to redirect flips/pulls (avoid conflicts)

• Implement the ros api above

• Use watchlists/blacklists for public/flipped/pulled interface

• Remapping of connection names from a flip/pull - cannot do (refer #62)