Major Revisions

[Simon-Cooper]

Latest revision of motion control code. New function block wrapping of PLCOPEN MC FBs pulled inside state machine logic. GearIn and GearInMultiMaster implemented to include coupling of limit switches from slave axes to master axes, these are fed in to the MC_Power Block.

A lot of new DUTs (Structures and Enumerations) added for simplicity: eRWParameter ST_MasterAxis ST_AxisControl etc...

Visualisation screens overhauled to allow scaling, also implements a basic (but hardcoded) slit set graphic.

MAIN POU includes state machine logic for implementing a Multi-master slit set. Main cleaned up to run through ACTS AXES() runs the FB_AxisDrive for the system PROG() runs custom system specific control - currently a slit set SIMULATE() runs axis simulation code for creating limits and flags

Advanced homing implemented but functionality not yet tested with hardware.

Things to test: Limit coupling of slaves to masters when gearing in and out, how robust is this. Scripting of other systems (not a slit set).

[DominicOram]

Look a lot better in general but as you said, this is a big change so a lot of comments here:

Functionality

• Out of the box the basic moving/coupling of axes seems fine but the slit set does not work. Does this require some extra setup from me?
• Axes 1-4 are in an error state and I get the following errors:

  Severity    Code    Description Project File    Line    Suppression State
  Error       19/11/2018 18:56:37 649 ms | 'TCNC' (500): R0 Management: Cannot couple axis 2 to axis 0 !              
  Error       19/11/2018 18:56:37 649 ms | 'TCNC' (500): Axis with ID 0 is unknown here !             
  Error       19/11/2018 18:56:37 649 ms | 'TCNC' (500): Axis with ID 0 is unknown here !             
  Error       19/11/2018 18:56:37 649 ms | 'TCNC' (500): R0 Management: Cannot couple axis 3 to axis 0 !              

• Honestly, this has reached the point where you know far more about how this is meant to work than I do. On the face of it seems to most of it seems to work as expected but it's probably best we look at functionality together. You also need to start writing more comprehensive documentation on how to do things with the system and how it works under the hood.

Code Structure

• In general we should be more contentious of what gets put into git and what doesn't, this change has added 200,000 lines, I suspect you didn't write this many lines of code. e.g.
  • There are 3 measurement projects, are these scopes that you anticipate developers continually using? If so they should be well named and documented as to their use. If they were just a one off for debugging, remove them
    • Similar with the *.csv test files. Document or remove
    • There are a huge number of compiled libraries in here, I don't think either of us are sure whether they belong here yet but there are a number where we have two versions of the same library. I've added a script that will tell you when these dependencies don't match the tpy file and deleted the unused ones.
    • The files inside _Boot look to be built files. We should definitely avoid having built files in the repo
    • The license file will be different for everyone, I've removed it from the repo.

VISUs

• I really like the state machine diagram, the interface in general is quite nice, however you should update (https://github.com/ISISComputingGroup/BeckhoffPLCCode/wiki/Introduction-to-using-the-Visualisation)
• I like the folder structure of the visualisation, I've extended it a bit to make it more obvious which screen is the 'top' screen and done a bit of renaming (which is probably more personal preference at this point). I've added a note here that reflects the convention you've started.
• We should have a standard for how visualisations should look e.g. the labels in some places have all caps, in others camel case. We have such standard in IBEX (there's even a script that checks we've got it correct) where we use sentence case for labels and title case for titles.
• It would be nice to have some idea of why an axis is in error on the visualisation
• Minor point, when the green buttons are disabled it's not obvious

Documentation

• A document on how to set up a slit set would be good (aim it as an idiot's guide), even better would be a script to auto gear one in for you, we'll have to think about that a bit
• The way the limit propagation works is probably worthy of a document with a nice diagram

Code Quality

• I don't like the AxisState stuff, it's a magic number that's not obvious and the axis.status already gives us the information in a reasonable form anyway ticket here
• In general it's bad practice to have commented out code, which you have in a few places (i.e. where things aren't yet implemented by Beckhoff or you've changed how something is done). Have a look at here for some reasons why.
• You've started your arrays at 1, most languages start at 0 (which I prefer), if this is a PLC thing I can accept it, but we should add to the standards.
• I've commented on individual lines where relevant
• I love the use of methods, use more! Can we leverage them in the FB_AxisDrive too?
• There's places where you've used comments to give yourself notes about things that you need to do etc. in general that's bad practice but given where we are with the project it's probably ok for now.

I wasn't really sure whether to reject this or accept this PR as it's such a large change. I won't merge for now and we'll sit down and discuss it when you're back. Sorry for the wall of text, this is honestly very good work. I'm just keen that it's as good as possible and that it can be easily picked up by others if required.