Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Multi-plc Support #6

Open
wants to merge 38 commits into
base: v0.2.0.rc1
Choose a base branch
from
Open

Multi-plc Support #6

wants to merge 38 commits into from

Conversation

Joshpolansky
Copy link
Member

@Joshpolansky Joshpolansky commented Oct 15, 2024 •
edited
Loading

What:

  • Add variable read data to the USD in the session layer
  • Add support for Multiple PLC's
  • Moves connection information from global persistent to USD based
  • Improves cycle time performance from (3 + write) x ads time + to 1x ads time
  • Decrease jitter in refresh rate
  • Refactor code to use common components with other loupe extensions

Why:

  • Interfacing with the USD means the user does not need to use any python to interact with the extension
  • Multiple PLC are common in digital twin
  • Saving connection information in the USD allows users to have multiple different projects. It is also supports multiple PLC's in a clean wau
  • The extension was limited to reading new data every 30 ms. If writes were done in the same loop it would drop to 40 ms. This was due to the connection check making an ADS calls, and that check being called twice per scan.
  • The calculation for the next read time included the sleep time of the thread. If the thread slept for longer than specified sleep time, the sleep time added to the jitter. Previously there was a typical jitter of 8-20 ms depending on system load. New jitter is typically on the order of 1 ms.

TODO

This PR does not update the tests or docs. They should be updated before release.

New configuration UI:
Snag_f6f61b

New USD structure:
Snag_f73670

Configuration saved in the USD:
Snag_f76c06

Variable reading/writing Structure
Snag_f7a650

@Joshpolansky
Update time calculations to reduce jitter
ae8dce0 
Also update UI spacing
@Joshpolansky
Run formatter
b742eca
@Joshpolansky
Update menu structure
153db62
@Joshpolansky
Updating to support multi-plc
03b79d9
@Joshpolansky
Fix some statuses
881e461
@Joshpolansky
Add a separate connection for writes
d8dcd0f
@Joshpolansky
Start extracting a base runtime class
26db5c0
@Joshpolansky
Updates based on review
9d72abc
@Joshpolansky
run formatter
97fbac7
@Joshpolansky
Fix dependency direction
d754d37
@Joshpolansky
Add some dropdown selections
3a7ce0c
@Joshpolansky
Add default PLC name
b8b1783
@Joshpolansky
Start UsdManager
783e545
@Joshpolansky
Add read variables to USD
3dfd4f9
@Joshpolansky
Start adding write support
3099d7f
@Joshpolansky
Re-org to allow for sharing code with other extensions
1680a8b
@Joshpolansky
[WIP] generalize the API
01be3a9
@Joshpolansky
refactor to be more modular
1fcd10a
@Joshpolansky
[WIP] Add input to setup cyclic read variables
ddd276a
@Joshpolansky
Fix write values on creation
45b6370
@Joshpolansky
Reorg ui
17c35e4
@Joshpolansky
Fix copy object
4d20969
@Joshpolansky
Add method to overwrite read variables
0c0b0c0
@Joshpolansky
Move usd manager edits to a user specified layer
89123a8
@Joshpolansky
move common to prepare for module
6686d4a
@Joshpolansky
Add common repo
e6dce7c
@Joshpolansky
Move files from this repo to a common repo for sharing
387cc1a
@sclaiborne
Copy link
Member

sclaiborne commented Oct 21, 2024 •
edited
Loading

Looks good in general, I have a little more review todo.

Is tools/packman suppose to be ignored (currently it is not)?

@Joshpolansky
Copy link
Member Author

Looks good in general, I have a little more review todo.

Is tools/packman suppose to be ignored (currently it is not)?

The tools folder was included intentionally for development. I suppose it's debatable if it should be there. Thoughts?

@sclaiborne
Copy link
Member

I have not forgotten about this, I am scheduling some time to review next week

sclaiborne
sclaiborne approved these changes Nov 11, 2024
View reviewed changes
Copy link
Member

@sclaiborne sclaiborne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, A couple small open questions.

exts/loupe.simulation.beckhoff_bridge/loupe/simulation/beckhoff_bridge/BeckhoffBridge.py
Comment on lines +13 to +14
from ..common.RuntimeBase import get_stream_name
from ..common.BridgeManager import BridgeManager, Manager_Events as ManEvents
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we decided common, needs to be renamed/moved here. Did we want to do that before merging?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, this should be fixed before we merge

exts/loupe.simulation.beckhoff_bridge/loupe/simulation/beckhoff_bridge/global_variables.py
default_beckoff_properties = {
ATTR_BECKHOFF_BRIDGE_ENABLE: False,
ATTR_BECKHOFF_BRIDGE_REFRESH: 20,
ATTR_BECKHOFF_BRIDGE_AMS_NET_ID: "127.0.0.1.1.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you referreing to? The AMS_NET_ID is sorta like a IP Address with 2 extra numbers, is that the question?

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

@sclaiborne sclaiborne sclaiborne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Joshpolansky @sclaiborne