PLCgen improvement plan
Development of PLCgen (see #397 (closed)) has progressed to the point that all main features are present and running. Issue #676 (closed) is tracking progress for making the program available for everybody.
That however does not mean development of the program ends with closing #676 (closed). There are a still many things left to do in the code generator. The main change of this issue compared to #397 (closed) is that instead of constantly building new functionality, the focus here is more at consolidating and improving the program to a mature tool.
Things to do
Preparation
-
Copy remaining relevant points of #397 (closed) to here. -
Integrate TODO
items currently sprinkled throughout the code. (#397 (closed), point 26) -
Add list of possible improvements from the report published by CGI for as far as they relate to the PLC code generator. -
Finish and close #676 (closed) -
Finish and close #397 (closed) with respect to all points that have not been copied to this issue.
Bugs
-
PLCgen Siemens target: location variables get assigned non-existing values. (#675 (closed)) (#397 (closed) point 31) -
Proper code generation for the Siemens target wrt timers can arguably also be seen as a high priority improvement/fix. See also the 'Targets' section below (#679). -
Protect the PLC from getting negative TON end-values. -
cifplcgen
is missing from the scriptable tools overview in the CIF documentation. (!731 (merged)) -
Issues found during testing of generated S7 PLC code (thanks to @jverbakel): -
State variables need a memory address when they are in a global table, or they can be stored in a DB file. (Fixed, they got moved to a DB file in the mean time.) -
Double underscore sequences in an identifier are not allowed (Gets fixed in !806 (merged)). -
firstRun
variable does not exist. Comment: Also check its initialization value. -
preset_timer_t
variable does not exist. -
timer
is a reserved word (it's a function block type in S7). -
Fights between local variable name, global variable name, and eg types can be resolved by prefixing local variables with#
and adding"
around global variables. (rejected, as not supported by all PLC vendors) -
PLC timers use TIME
type, while CIF uses real numbers to express duration. Conversion is needed with a cast from a number toTIME
, eg bytimer0.PT := DINT_TO_TIME(LREAL_TO_DINT(#preset_timer_t * 1000));
. CIF does not prescribe a unit fortime
, but generally we use 'seconds' as unit, and the PLCgen docs describe this already as well. -
SEL
function expects boolean values to select, other types must be denoted by appending a type, e.g.SEL_LREAL
.
-
Conceptual cleanliness
-
Reconsider how a function block call should be created. Currently, the code that produces calls to the timer function block may have too much knowledge about PLC target information. See !683 (comment 1320599), !688 (comment 1349579) and !770 (comment 1700315). -
!785 (merged) tried and failed in the current setup of variables and functions. However, as noted in !785 (comment 1704688) the problem could be that variables and functions are much closer related than they are usually in other languages. The direction therefore could potentially be to merge these concept in the core data (possibly with interfaces), and then variables, functions, function blocks, POUs, and programs all collapse to the same basic idea. -
Reconsider the model classes for functions, function blocks, function calls, variables, etc. See !785 (comment 1704170). (#770)
-
-
ExprGenerator
has a bunch of// S7-400 and S7-300 only support <<OPERATOR>> on the same types.
It ensures these restrictions without regard to the actually used target. Other comments seems to point at similar considerations, check all line comment in that file. -
Variable tables have confusing names. Method addTimerVariable
is used to add non-timer variables. Reconsider it all. See also !697 (comment 1392717). Fixed in !770 (merged). -
As suggested at !770 (comment 1658164), cleaning up variable access with respect to getting the name or getting the stored value of the variable is useful. Fixed in !785 (merged).
Industrial usage
A Dutch report (PDF) was written by CGI for RWS about PLC code generation in synthesis-based engineering (SBE) context.
Many conclusions are about the design process, rather than about implementation issues in the generator itself. Here, only the latter are considered.
The identified points have been given a brief English description. The V
and A
references relate to points in the report.
The V
references are safety related, while the A
references are more general considerations from system development.
-
Ensure suitably short PLC cycle execution times ( V-3
). (see #621 for event loop bound calculation) -
Avoid using the WHILE
statement (V-4
). -
Add suitable comment lines to the code what it is doing ( V-6
), egread inputs
ortry to perform event E
. (#782 (closed) adds comments to the code) -
Add more details to input and output code about what is read or written. -
Add more details to initialization of state variables, and updates of continuous variables. -
Extend the code generator to include safety PLCs ( V-1
,A-7
). -
Avoid set
/reset
function blocks (V-7
). -
Include the used versions of the input models, synthesis software, and code generator in the generated code ( V-10
). (#805 allows inserting a text-file at the top of the code) -
Allow function blocks to be included from a model ( A-2
). -
Allow having user-parameters to tune/configure system behavior without falling back to changing the models ( A-3
). -
Enable relating between modeling information and the generated code ( A-4
,A-13
). #587 is a first step in this direction, #593 is working on the underlying foundation to enable such steps.-
A-4
specifically addresses the need to relate generated guard tests for a requirement, while -
A-13
addresses the reverse need (which requirements are addressed in this code?) -
See what else is need to ensure traceability.
-
-
Ensure that small changes in the models don't lead to large changes in the generated code ( A-5
).
Improve user-experience (UX)
-
Add plc:input
annotations for linking sensor input to CIF variables. #593 -
Add plc:output
annotations for linking actuator output to CIF variables. #593 -
Add doc
annotations for attaching documentation links to elements in the CIF specification. #593 #587 (#397 (closed) point 28) -
Add user-defined internal functions (#397 (closed) point 18). -
Add continuous variables (#397 (closed) point 6, #630 (closed) implements it ) -
Prechecker "continuous variable as timer" sometimes emits strange error reports, see #681 (closed) (!798 (merged)) (!819 (merged))
Pre-checking
-
Consider generalizing the 'continuous variables as timers' check and move it to common.cif.precheckers
. (#397 (closed) point 31) -
Implement non-blocking under control checking, described in Reijnen 202, needed by plcgen, may possibly be done independently (#397 (closed) point 25, #410).
Testing
-
Reduce test duplication. (#781 (closed)) -
Ensure all cif2plc
tests are also added forplcgen
. (could be part of test set in #798) -
Test that all options have the desired effect on the generated code, for all targets. (should be part of test set in #798 for S7-300) -
Perform testing for what function notation forms are supported by what target, and update the target classes to match that. (should be part of #798 for S7-300) -
Perform extensive testing of the new PLC code generator, on real PLCs, for the various targets. (should be part of #798 for S7-300)
Options
-
Check which options are really useful. Maybe some could be removed and just decided by the target instead. (See #679 (comment 1657463) for the argumentation that why it is considered done.) -
Add "auto" to the convert-enums option to let the target decide. (!764 (merged)) -
Check and handle other options that lack an auto
value. (See #679 (comment 1657458) for the argumentation why it is considered done.) -
Enum conversion should handle three output forms: Enumeration values, constants, and numbers. -
Consider renaming options, but only if we agree that is a good idea. See #679 (comment 1392038). -
Consider a different name for convert-enums
, such asconvert-locations
, to improve clarity in what it does? -
Consider a different name for simplify-values
to avoid confusion withconvert-enums
functionality?
-
-
Replace the PlcFormalFuncInvokeArgOption
,PlcFormalFuncInvokeFuncOption
options by 'supported function invocation notations'. (!688 (merged)) -
Remove the PlcFormalFuncInvokeArgOption
,PlcFormalFuncInvokeFuncOption
options. (!697 (merged)) -
Add the PlcMaxIterOption
options. (!697 (merged)) -
Output file option is not always for creating a file.
Code improvements
-
Consider generating names in code more carefully so there is a system in place. See also !664 (comment 1262087). -
Consider adding an analysis+rewrite step to improve generated PLC statement models (from #397 (closed) point 8, and several other sub-points so it covers all available expressions). -
Consider improving algebraic variable handling beyond replacing them with their definitions (#397 (closed) points 21/22). Perhaps with the analysis framework? -
Implement constant declarations and usage in the PLC (from #397 (closed) point 3). -
Code still has some root[Cif]Provider
names, they should be renamed toscopeCifProvider
. (#397 (closed) point 31) (!751 (merged)) -
Currently there is a requirement "The continuous variable is only assigned in a single-variable assignment, not in multi-assingments.". This seems an unnecessary limitation. (#97 point 31) -
Reduce restrictions for precondition check on continuous variables: -
Checker currently requires 'Continuous variables, if explicitly initialized, must have an initial value that is zero or larger, and this must be statically decidable.' We may want to reconsider this. See also !700 (comment 1477524). -
Checker currently requires continuous variables to be assigned only in single-variable assignments, while the implementation does support multi-assignments already. See also !700 (comment 1477527). -
Checker currently requires continuous variables to be initialized to, assigned, or compared to, a non-negative value. Not all of these restrictions may be needed. See also !700 (comment 1517084). -
Remove app.framework
paths from the writers. See #666.
-
Event transitions code improvements
-
In event transition code magic numbers are used to point at automata and edges. It should likely use CIF automata and CIF location names instead. (#397 (closed) point 29) (#774 discusses names and number triplets like 12_34_456
(automaton, location, edge) ) -
Event transition code could use more comments what is being done to guide the reviewer. For example -
mention the automaton that owns the edges being checked. (#782 (closed) adds comments to the code) -
Some empty lines eg between different events could be useful too. (#397 (closed) point 29) -
other improvements if needed/useful.
-
-
Use better names or add comments to make it easier to find the connection between guard checking code and update code of the same edge. (#397 (closed) point 29) (#782 (closed) adds comments to the code)
Performance
-
Add sequencing of event transition processing (Reijnen 2020, #397 (closed) point 23): -
Collect querying of state data in guards and state updates. [See issue #595 ] -
Construct a graph as input for the algorithm.
-
Targets
-
Add a timer type-name ( IEC_Timer
for S7). -
Allow for a function name suffix ( .TON
) with a function-block instance. (!785 (merged) ) -
Consider integrating Writer
classes more withTarget
classes (#397 (closed) point 27) -
Targets could perhaps also use a fresh look at their code (#397 (closed) point 27). -
S7
(!770 (merged), !785 (merged)) -
ABB
-
PlcOpen XML
-
TwinCAT
-
IEC 61131-3
-
-
Siemens target currently banishes all forms of array, but as PLCgen relies less on functions, the reason for that in cifplc (".. because S7 doesn't support functions returning arrays and doesn't support arrays of arrays.") seems to leave room for improvement. -
Try to add links to public PLC system documentation for reference for the targets (see !770 (comment 1657881) ) -
PLCopen XML
-> https://reference.opcfoundation.org/nodesets/87 -
IEC 61131-3
-> No public access, Wikipedia has a short description https://en.wikipedia.org/wiki/IEC_61131-3, likely other targets are useful too. -
TwinCAT
-> https://infosys.beckhoff.com/english.php?content=../content/1033/tc3_plc_intro/2529458827.html&id= -
ABB
-
Siemens S7-300
-
Siemens S7-400
-
Siemens S7-1200
-
Siemens S7-1500
-
Other
-
Improve naming in the generated code, and add documentation that explains the generated code. (#774) (#782 (closed) adds comments to the code) -
CIF assumes that both input and output variables are part of the state. This is debatable as often these variables are only pass-through copies of the read input data or created output data. (#397 (closed) "other considerations" at the bottom) -
Since all code and data lives in the main program, there isn't any need to make a copy of the inputs to a state variable. It can simply always use the input variable itself instead.- (As noted in #679 (comment 1656095), using the input variable itself may at some point interfere with input changes due to actions from the asynchronously running safety PLC.)
-
Another form of this improvement can be not use a state variable for storing read input. That value is valid only for the duration of the cycle, and could be a temporary variable instead.
-
-
As noted in !619 (merged) (comment 1159022) the chosen approach of performing event transitions by first deciding complete feasibility of an event before even considering performing it means that guard checking code of an edge and the update code that the edge may perform are not close to each other in the source code. This complicates code review. (#397 (closed) "other considerations" at the bottom) -
#628 should be decided at some point. (#397 (closed) "other considerations" at the bottom) -
See what else to do to address the recommendations from the report that CGI wrote for RWS about PLC code generation: ROK-IA.SBE.RAP01_v1.0.pdf
Migration
-
Does it support all tool options, properly. (#397 (closed) point 32) -
Does it have all the features of the current/old PLC code generator? Is it a full replacement? (#397 (closed) point 32) -
Is there any documentation, are there test, etc, from the current/old PLC code generator that have not yet been migrated. (#397 (closed) point 32) -
Deprecate cif2plc
. -
Get rid of cif2plc
,cif to plc
,cif_to_plc
, etc in PLCgen.