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

Switch from modbus to OPC-UA for MPS communication #32

Merged
merged 142 commits into from
Jun 4, 2019

Conversation

morxa
Copy link
Member

@morxa morxa commented May 20, 2019

Switch to OPC-UA and a new communication protocol for the communication with the MPS machines. In particular:

library mps_comm

Implement OPC-UA communication in the new library mps_comm. The library provides commands such as conveyor_move and get_base, which are provided to CLIPS by function bindings. All commands are processed in a command queue by a separate worker thread. The worker thread waits 40ms after sending a command to make sure that the MPS does not get two commands within 20ms (in which case the first command would be ignored). Commands are run non-blocking, i.e., we do not wait for the MPS to complete the command. Instead, also register callbacks for the BUSY and READY flags, which can then be processed by the refbox (in CLIPS).

The new library supports three connection modes for an MPS:

  • MOCKUP: Do not actually connect to any endpoint, but simply mock commands by blocking for a fixed amount of time.
  • SIMULATION: Connect to a simulated MPS, which allows setting the registers from a web interface. I've never used this personally, but it was used by @maestrini to develop the OPC-UA communication.
  • PLC: Connect to a PLC on an actual machine.

The modes can be configured, see the provided config for an example.

For the actual connection to the MPS, use freeopcua. Note that currently there is no Ubuntu package for that library. The circleci config is adapted accordingly to build with freeopcua on Fedora, and to not fail due to the missing library on Ubuntu.

Refbox changes in CLIPS

Implement the new communication logic in CLIPS, which requires a number of changes to the production code. We now instruct a machine for each step, e.g., first move the conveyor belt to the middle, then retrieve a cap, then move the workpiece to the output. The pattern is similar for each MPS, e.g., for a cap station:

  1. When receiving a prepare, start the conveyor belt.
  2. Wait for the machine to become BUSY and then not BUSY. Set the machine to PROCESSING.
  3. Instruct the MPS to mount/receive a cap.
  4. Wait for the machine to be not BUSY. Set the state to PROCESSED.
  5. Move the workpiece to the output.
  6. Wait for the machine to be READY. Set the machine to READY-AT-OUTPUT.
  7. Wait for READY to be FALSE. Set the machine back to IDLE.

Along the way, get rid of the slots proc-state, mps-state, and mps-state-deferred. The MPS no longer has a separate state, we track everything on our side now. The slot proc-state became unnecessary due to the other changes. Also get rid of most of the simulation code, as this is no longer necessary and replaced by the mockup functionality of mps_comm.

This resolves #14 and supersedes #21.

Co-authored by @maestrini.

morxa and others added 30 commits April 22, 2019 15:24
The machine may be ahead and may have skipped over the PREPARED state
already. Thus, also stop sending prepare messages if the machine is
PROCESSING/PROCESSED.
We want to use the new communication protocol without a relay, thus the
communication library should be directly in libs.

This does not switch any consumers to the new communication.
We do not use CamelCase but names_with_underlines.
- Makefile:
  - Added freeopcua relevant libraries to libllsf_sps
  - Added new files to build chain

- machine.h:
  - TODO: revert back: replaced '<' with '"' for msgs/MachineInstructions.pb.h and added link; did not work without this change
  - TODO: timeouts *10 for testing and stability reasons; needs more knowledge about the system
  - Removed modbus.h include
  - Added opc ua relevant code as protected

- mps_io_mapping.h
  - Added two additional status bits that exist in new sps

- opc_utils.h & opc_utils.cpp contains all code regarding freeopcua

- subscription_client.h handles the value changes for subscribed registers

- ring_station.h: small changes, so it also reads the barcode register
added missing functions to 3 sps types (base_station, cap_station and ring_station) to send band to in, middle and out

changed MSGS file path to relative

added 64 bit code for 64 bit mps simulation
Callback is called whenever subscription client receives a new value.
An example usage can be found at machine.cpp at line 311.
Those enums are only available in the message definition that uses the
protobuf relay. As we do not use the relay, we don't use those messages.
Instead, just define new enums.
This does not completely implement the new OPC-based communication but
merely adapts the existing functions to use the new mps_comm library.
The refbox interface no longer manages each MPS as a thread, because the
Machine definition in the new OPC-based libmps_comm is not thread-based.

Instead, we will need to manage asynchronous operations differently.
We can have only one logging client with the same name. Use the MPS name
as logging client.
The Machine now needs its name, pass it along.
We cannot implement this yet because we do not have the respective
function in mps_comm. Re-add it as mockup because we need the function
in CLIPS.
Instead of running MPS operations synchronously, create a std::future
and start it asynchronously. The future will assert a CLIPS fact once
the operation has finished.

Only do this for MPS operations that actually take time.
The function mps-ds-process now instructs the DS asynchronously, i.e.,
it immediately returns and an mps-feedback fact is asserted when the DS
finished. Adapt accordingly by setting the machine to PROCESSED when we
received the mps-feedback.
We do not necessarily have a workpiece available when the machine is
prepared, so do not touch the mps-state when processing a prepare
message.
Calling mps-bs-dispense just triggers the dispense and we receive
asynchronous feedback for the success of the operation. React on that
feedback by setting the mps-state to DELIVERED.
Directly call mps-rs-mount-ring when the machine was prepared. This
assumes that the belt keeps running until it detects a workpiece in the
sensor, i.e., the belt possibly runs before a workpiece was put into the
machine.

After the ring was mounted, deliver it to the output of the RS and set
the machine to READY-AT-OUTPUT.
Directly call mps-cs-process when the machine was prepared. This assumes
that the belt keeps running until it detects a workpiece in the sensor,
i.e., the belt possibly runs before a workpiece was put into the
machine.

After the CS operation finished, deliver the workpiece to the output of
the RS and set the machine to READY-AT-OUTPUT.
These saliences are clearly not necessary, remove them.
@morxa
Copy link
Member Author

morxa commented May 28, 2019

Thanks for the comments!

So I have carefully gone through the changes in production.clp (in parallel with and after re-reading the old production.clp to have it fresh in memory)

I have to say, not only is the flow is much much clearer now, many bugs and workarounds form that past are also remedied. Thank you for that. Great work indeed decoding that to this!!

Yet in going through it, I encountered some minor styling and readability issues that I think could be improved to help get the gest faster. I will point those out shortly with the labeled "suggestion" or "styling" since its not crucial to the functionality and could be added later (even though I think it would be better to handle them sooner)
Those mainly are:

* Many of the useful printing is either gone or not added

I deliberately removed some output because it was redundant. What exactly are you missing?

* Sometimes the order of the rules seems a bit random which causes to keep jumping up and down to find related rules (I suppose this is due to the speed and incremental nature of RoboCup)

Most rules are ordered by mps-type. As some rules apply to multiple types, they may seem out-of-place, but that shouldn't be a big issue. Do you have a specific suggestion for reordering?

* Some of the documentation and the names of the rules could be improved

Done.

Follows, please find my change requests and inline minor suggestions

I also removed a redundant rule and silenced the output by OPC-UA a bit.

This removes light signals, heartbeats, and resets from the log output.
@morxa morxa force-pushed the common/opc-ua-mps-communication branch from e529ac0 to eba48f2 Compare May 28, 2019 13:46
Copy link
Member

@MostafaGomaa MostafaGomaa left a comment

Choose a reason for hiding this comment

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

Thanks for the quick reply!

I deliberately removed some output because it was redundant. What exactly are you missing?

sure makes sense, I also did comment on any missing prints for now as its really only important for tracking. I will check and post in line for those that I missed.

Most rules are ordered by mps-type. As some rules apply to multiple types, they may seem out-of-place, but that shouldn't be a big issue. Do you have a specific suggestion for reordering?

I have added inline comments for most of those. but to sum up

  • Grouping scattered rules that react on mps-feedback
  • Grouping scattered light rules and consistent naming
  • Placing the general PROCESSED to READY_AT_OUTPUT after all the mps PROCESSING
  • Placing the general READY_AT_OUTPUT to WAIT-IDLE/IDLE after that
  • Afterward, all the rules are already in place in the flow. (ie, broken rules and then timeout)
    hope am not forgetting anything. That is just my suggestion, please feel free to see the grouping u see fit

Done.

Cool and thanks for the fast reacting

There is a generic version of this rule that handles all machines, thus
there is no need for a special rule for the BS/RS.

Co-Authored-By: Mostafa Gomaa <[email protected]>
TarikViehmann
TarikViehmann previously approved these changes May 28, 2019
Copy link
Contributor

@TarikViehmann TarikViehmann left a comment

Choose a reason for hiding this comment

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

I looked over all stuff again and it seems solid to me. @MostafaGomaa covered the remarks i had regarding styling with his detailed feedback (awesome work!), so i approve this.

morxa and others added 2 commits May 28, 2019 20:51
Move the code around so all the mps-status-feedback processing rules are
together.

Co-Authored-By: Mostafa Gomaa <[email protected]>
Move all light signal changes into separate rules. Move those rules into
a separate files. This avoid duplication and inconsistencies, which may
occur if the lights always need be set when the machine state is set.
Instead, always synchronize the light signal with the machine state.
morxa and others added 8 commits May 28, 2019 22:23
The machine type can never be BS because we excluded that case.
This better fits the control flow.

Co-Authored-By: Mostafa Gomaa <[email protected]>
We should always only instruct a machine if we are in PRODUCTION and the
current state is RUNNING. Add a guard where this was not in place.
The timeout may actually be caused by the team, e.g., if a robot
places the workpiece on the edge of the conveyor belt. Thus, handle this
like the other case, where no product is fed in time. If it is actually
a machine failure, the machine should still break, so the referee can
clear the machine.

Also use proc-start for the timeout instead of wait-for-product-since.
To do this, set proc-start wherever we set the state of the machine to
PROCESSING.
We do not need the previous state if the machine is BROKEN, because the
machine will be reset afterwards anyway. Only use it for a machine with
state DOWN, because in this case, the machine needs to continue where it
was previously.
@morxa
Copy link
Member Author

morxa commented May 29, 2019

Thanks for the thorough reviews! I tried to tackle all concerns, please review again.

@MostafaGomaa
Copy link
Member

Thank you for the quick and rather precise edits. I will take another look asap

@MostafaGomaa
Copy link
Member

Other than the pending comments about the WAIT-IDLE status I have no further comments about this PR.
I will gladly approve this PR once that is resolved.
Thank you very much for your effort making this happen and neatly realizing the changes arising during reviewing of the PR :)

morxa added 2 commits June 4, 2019 13:57
Switch the machine to WAIT-IDLE before it goes to IDLE. After 3 seconds,
proceed to IDLE.
@morxa
Copy link
Member Author

morxa commented Jun 4, 2019

Other than the pending comments about the WAIT-IDLE status I have no further comments about this PR.
I will gladly approve this PR once that is resolved.
Thank you very much for your effort making this happen and neatly realizing the changes arising during reviewing of the PR :)

I re-added WAIT-IDLE. Please have another look!

Copy link
Member

@MostafaGomaa MostafaGomaa left a comment

Choose a reason for hiding this comment

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

Thanks again for the changes! Looks pretty good to me

@morxa
Copy link
Member Author

morxa commented Jun 4, 2019

Thank you for the detailed review!

@morxa morxa merged commit 36ee8d4 into master Jun 4, 2019
@morxa morxa deleted the common/opc-ua-mps-communication branch June 4, 2019 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refbox OPC-UA communication
3 participants