Skip to content

Refactor connection lifecycle for asynchronous sessions contexts#27

Open
michaelosthege wants to merge 5 commits intoCommonplaceRobotics:masterfrom
JuBiotech:contextmanager-refactor
Open

Refactor connection lifecycle for asynchronous sessions contexts#27
michaelosthege wants to merge 5 commits intoCommonplaceRobotics:masterfrom
JuBiotech:contextmanager-refactor

Conversation

@michaelosthege
Copy link
Contributor

This replaces #26 where I tried to work around an issue that I ran into due to unclean connect/disconnect/dispose resource management when connections failed.

Essentially the threads must be regarded as "single use", but this requires them to be recreated if the same CRIController should be re-used.

Even more convenient would be a context manager that deals with proper disconnects and resource disposal, regardless of user level implementation.

Based on my comment here I refactored in the following way:

  • connect is changed to raise on errors -- this is a breaking change, but it's much easier to work with as it enables passing contextual information up the stack (through exception types and messages) which is not the case with return False.
  • CRIController class is split into CRIClient and CRIController(CRIClient) which makes it possible to separate observational and actuating code based on type information.
  • CRIConnector is introduced: This is a factory-like creator of connection session contexts.
    • The context managers deal with resource disposal, which simplifies usage.
    • They are already implemented as async context managers.

I also added an example that uses the CRIConnector contexts.

This asynchronous context manager style connection is relevant for us for multiple reasons:

  • We connect (asynchronously) to OPC UA servers in parallel
  • We connect to multiple iRC controllers
  • CRIClient can be long-lived, auto-reconnectiing to monitor while CRIController is only invoked for movement
  • All this happens in a FastAPI app which needs to remain responsive

What do you think, @cpr-bar ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant