oysstu / pyimc

Python bindings for Inter-Module Communication Protocol (IMC)
Other
9 stars 6 forks source link

WIP: Adds option to set a custom service to subscribe to. #2

Open krisklau opened 7 years ago

krisklau commented 7 years ago

Hi,

I would like the option to specify which service I would like to subscribe to in my actor. By using the functionality exposed in this (for now unmerged PR) https://github.com/LSTS/dune/pull/95, one can set up a seperate Transports.UDP with a service of e.g. imc+udp+pylive, which enables the python interface to only receive the messages it needs at a desired rate.

An example usecase is raw sensor-data you would like the full data-rate stream, but do not want to bog down the link to CCUs such as Neptus.

Consider this work-in-progres. This is functional, but needs some improvements:

Thanks, Kristian

oysstu commented 7 years ago

If I understand this correctly from the DUNE pull request, it is simply a name given to a regular UDP transport (either static or dynamic), and can (optionally) be used for rate limiting? I think it's a fair suggestion to add this. I'm leaning towards adding it as a preferred service though, with a graceful fallback to imc+udp if possible (with a warning message). This same interface can also be used to select imc+tcp in the future (when/if implemented) with a fallback to udp. A separate function for selecting the service on a per-message basis can also be added (for advanced usage where multiple transports are used).

As for being spammed by Neptus, I think a feature separate to the above should be added to filter nodes based on the announced type. This should be based on a blacklist rather than a whitelist, to account for added types in different IMC defs.

Since this has been a single-man project so far, there are no fixed conventions. I follow the PEP guidelines to the best of my ability (a static code analyzer helps in following these). I also try to add typing information for all fixed types, at least for the classes/functions that users of the library interact with.

Øystein

krisklau commented 7 years ago

Hi,

Almost, I'll try to explain a bit better :)

In the default/normal UDP.Transports task, you typically set it up if expecting to be talking to a GCS. This means that all control/management messages are sent, but you would typically limit the amount of raw sensor data transmitted to save link bandwidth. For instance, you would not need 50Hz acceleration values, but e.g. 2 Hz is nice to verify data availability (this is acieved with the parameter Limit Rates).

However, for another interface, lets say my Python actor, I am especially interested in raw sensor data, to either do some testing of algorithms or do some real-time plotting. In this one, I do not care so much about vehicle management messages.

So to make sure Neptus connects to the default Transports.UDP task, we leave this with the service imc+udp. But for the Python Actor link, we start a seperate Transports.UDP task, with the specified/nescessary messages, with the imc+udp+pylive service. Then we know both neptus and Python will connect to the right service, and get the data it needs in the rate it needs.

I will do some more work on this PR and let you know when I make the improvements.

Best, Kristian

oysstu commented 7 years ago

Right, so it's a way to distinguish between services in the announce. One issue I have with the current approach is that it forces all connected nodes to only communicate over that service. I'd prefer an implementation that allows communication both with nodes not running the target service as well as any other imc+udp service. The preferred service approach I outlined would work for this.

Edit: A separate send function with explicit selection of service can also be a good idea in the future, if multiple services are to be used for the same node.

Øystein

krisklau commented 7 years ago

Hmm, I see your point. However, one should then be sure that the actor only receives data from a single remote node at the time (even if more are available), otherwise you could get duplicate messages from each service.

To still be able to connect to multiple nodes, maybe this can be an argument in the subscribe-function, or that a parameter is passed into the callback-function specifying source remote service type?

Personally I still would like an option to disable all other communication than the service I specify to save link bandwidht and loop-time in python. So a list of services (in preferred order) could be a nice solution.

KK

oysstu commented 7 years ago

I like your suggestion of having a list of preferred services. I'll look into it when I have time, rather busy at the moment. I'll merge a PR from you if you get around to it before me