Skip to content

Add asyncio support #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

Conversation

sveinse
Copy link
Collaborator

@sveinse sveinse commented Mar 27, 2023

This PR adds support of asyncio to canopen. The overall goals is to make canopen able to be used in either with asyncio or regular synchronous mode (but not at the same time) from the same code base.

Note that this work is still work in progress. This PR was created to discuss the specific solutions for async and non-async as mentioned in #272. This PR closes #272.

Current status until feature complete:

  • Implement ABlockUploadStream, ABlockDownloadStream and ATextIOWrapper for async in SdoClient. Not needed
  • Implement EcmyConsumer.wait() for async
  • Async implementation of LssMaster Only fast_scan
  • ~Async implementation of BaseNode402~Omitted for now
  • Implement async variant of Network.add_node()
  • Update unittests for async
  • Update examples
  • Update documentation

@sveinse
Copy link
Collaborator Author

sveinse commented Mar 27, 2023

What is the best way to deal with Variable attributes? The feedback from #355 indicates that it is desirable to keep the attr based get/set mechanisms. However, this scheme cannot be used for async as there is no await mechanism built into @data.setter.

    var = param.data   # Non-async use
    var = await param.aget_data()   # Async use

My goal has been to keep the async and non-async uses of canopen as equal as possible, but this is an area where users will see a difference.

@acolomb
Copy link
Member

acolomb commented Apr 25, 2024

There is an implementation for .read() and .write() on the underlying canopen.variable.Variable type. Could that be used in an async wrapper, or something similar added for async?

@sveinse
Copy link
Collaborator Author

sveinse commented Apr 25, 2024

@acolomb I'm not precisely sure what you mean, so let me guess: You need separate async methods for read and right and they should only call their respective async setters/getters.

@acolomb
Copy link
Member

acolomb commented Apr 25, 2024

Sorry, I was trying to answer your question in the previous comment about synchronous getters / setters used in the properties. What I meant was that instead we already do have methods to access a variable remotely. Those might be a better fit to mirror to the async world:

value = sdo_var.raw  # This is the usual, terse style recommended in the docs
value = sdo_var.read(fmt='raw')  # This also works already
value = await sdo_var.aread(fmt='raw')  # Feels like a natural enhancement for async

Note that the fmt='raw' argument can of course be left out, it's only needed for phys or desc variants. This would give us an easy way to support async without changing the existing synchronous property-based access.

@sveinse sveinse marked this pull request as draft April 26, 2024 05:31
* Move loop init to the Network
* Remove unused callbacks in LocalNode
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 46.56716% with 179 lines in your changes missing coverage. Please review.

Project coverage is 69.03%. Comparing base (ae71853) to head (46f9b4a).

Files with missing lines Patch % Lines
canopen/pdo/base.py 41.07% 60 Missing and 6 partials ⚠️
canopen/nmt.py 22.72% 33 Missing and 1 partial ⚠️
canopen/variable.py 41.46% 24 Missing ⚠️
canopen/sdo/base.py 44.44% 15 Missing ⚠️
canopen/emcy.py 33.33% 12 Missing ⚠️
canopen/node/remote.py 50.00% 4 Missing and 2 partials ⚠️
canopen/sdo/client.py 45.45% 6 Missing ⚠️
canopen/network.py 76.19% 2 Missing and 3 partials ⚠️
canopen/sdo/server.py 66.66% 3 Missing and 1 partial ⚠️
canopen/async_guard.py 85.71% 1 Missing and 1 partial ⚠️
... and 3 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
- Coverage   71.36%   69.03%   -2.34%     
==========================================
  Files          26       27       +1     
  Lines        3129     3397     +268     
  Branches      480      523      +43     
==========================================
+ Hits         2233     2345     +112     
- Misses        765      906     +141     
- Partials      131      146      +15     
Files with missing lines Coverage Δ
canopen/node/base.py 100.00% <ø> (ø)
canopen/objectdictionary/__init__.py 83.33% <100.00%> (ø)
canopen/objectdictionary/eds.py 86.89% <100.00%> (+0.07%) ⬆️
canopen/profiles/p402.py 36.46% <ø> (ø)
canopen/lss.py 44.27% <90.00%> (+2.51%) ⬆️
canopen/async_guard.py 85.71% <85.71%> (ø)
canopen/sync.py 81.81% <60.00%> (-7.66%) ⬇️
canopen/timestamp.py 88.88% <60.00%> (-11.12%) ⬇️
canopen/sdo/server.py 86.95% <66.66%> (-2.02%) ⬇️
canopen/network.py 87.91% <76.19%> (-2.03%) ⬇️
... and 7 more

... and 1 file with indirect coverage changes

Copy link

codecov bot commented May 4, 2025

@sveinse sveinse force-pushed the feature-asyncio branch from 7486445 to 751f854 Compare May 15, 2025 15:52
sveinse added 6 commits June 12, 2025 00:56
* Make code more similar the upstream code
* Implement missing async functions
* Update README.rst and example
* Revert test cases to be diffable with upstream
* Ensure all skipTest() have useful messages
* Wash FIXMEs and NOTEs
* Adding `AllowBlocking` for temporary pausing the async guard
* skipTest() cleanup
* Increase test coverage
* OD object lookup issue
* SDO testing warning issue
* Fixed uncovered bugs
* Bumped minimum py version to 3.9 (due to asyncio compatibility)
* Added tests for PDO to increase coverage
@sveinse sveinse changed the title Add asyncio support [WIP] Add asyncio support Jun 15, 2025
@sveinse sveinse marked this pull request as ready for review June 15, 2025 13:50
@sveinse
Copy link
Collaborator Author

sveinse commented Jun 15, 2025

This branch is ready for review and integration into the project. Its not fully complete (I suppose it never will). The biggest lack is that there is no documentation for async.

The branch's README (https://github.com/sveinse/canopen-asyncio?tab=readme-ov-file#difference-between-async-and-non-async-version) contains an overview over the current differences between this branch and the main project.

The port is quite "naive" async-wise as it relies on asyncio.to_thread() on many async operations, which not ideal. The back-end of the canopen library relies on synchronous callbacks which doesn't work too well from an async perspective. To be able to support both non-async (regular) and async use, this approach have been chosen. In time I hope we can refactor the inner working of the library such that we can do it independent of the communication (e.g. vis sansio).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add async support to canopen
3 participants