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.

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

ENH: proof-of-concept for opening a local port #1

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

Conversation

klauer
Copy link

@klauer klauer commented Oct 26, 2023

Purpose

This is a proof-of-concept for allowing ADS clients to talk to localhost by way of requesting a unique AMS/ADS port from the ADS router itself.

Preliminary success

Paired with pcdshub/twincat-ads#12 and ads-ioc, these changes allowed for an unmodified rhel7-x86_64 (Linux) ads-ioc binary, running on TwinCAT BSD with the LinuxEmu layer enabled, to successfully communicate with localhost and serve PVs. 🎆

Sample PLC project and IOC used to verify this:

Note

Note that we (at PCDS) are way behind the upstream Beckhoff/ADS. As the upstream has significantly diverged from our fork, this effort would need to be redone entirely were we to ever sync/upgrade.

Changes

A few changes were necessary:

  • The ADS library counted ports from presumably a "free" area (30,000-65535). This was redone to track all ports from 0-65535, which certainly consumes a bit more memory yet also makes the array indexing logic markedly easier.
  • A new exported function AdsPortOpenLocalEx was added to perform this request. It needs to be called prior to any other ADS calls when the client recognizes that it is talking to localhost.
    • This is assuredly not the best place for it design-wise - the functionality as far as I can see doesn't really fit into the ADS library at all (but I would be happy to be proven wrong here; so feel free to comment).

Prior to this PR

Prior to this PR, the ADS library picks a port on its own and continues with it.

  • When opening a port in the "router" built into the free ADS library (by way of AdsPortOpenEx() ) merely reserves an integer AMS port ID locally and uses that for future communication ("request port ID, use that port ID to talk to AMS Net ID 8.8.8.8.1.1 port PLC_TC3 and read data from it").
  • This is reflected in this code:

    ADS/AdsLib/AmsRouter.cpp

    Lines 86 to 96 in 743a15a

    uint16_t AmsRouter::OpenPort()
    {
    std::lock_guard<std::recursive_mutex> lock(mutex);
    for (uint16_t i = 0; i < NUM_PORTS_MAX; ++i) {
    if (!ports[i].IsOpen()) {
    return ports[i].Open(PORT_BASE + i);
    }
    }
    return 0;
    }
  • It's effectively a free-for-all of port selection - "this application is free to reserve whatever ADS port it wants because there's no PLC running, no ADS router running, etc."
  • That local port is used in the AMS-over-Ethernet header as the source and the target PLC responds back to that port in requests.

Beyond a prototype

A few things to move this out of the prototype stage would include:

  • Remove the Recv() startup delay and replace it with some better thread-safe mechanism to only wait when necessary (or perhaps build the "wait for local port packet" into the AmsConnection class somehow)
  • Removing/lowering the warning level of all the debug prints
  • Remove the local AmsPort bool because I didn't end up using it
  • Adding the ability to free/deallocate the port from the PLC (see my comment links for reference here Run ADS Client on WSL through localhost Beckhoff/ADS#190 (comment))
  • Were this used to talk to multiple PLCs (localhost and one or more non-local hosts), the localhost connection would have to come first. This may need a workaround.
    • Expanding on that - if "localhost port open first" order is not followed, this scenario is possible:
      • To communicate with a non-localhost PLC, an arbitrary AMS port is chosen by the ADS lib by way of AdsPortOpenEx(): port X
      • AdsPortOpenLocalEx() is called to communicate with localhost, and this port number is identical to that from the previous step (somewhere around a probability of 1/65535....)
      • This port is already in use in the router and problems could potentially arise

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.

1 participant