Closed ZLLentz closed 4 years ago
The logical terms "TRUE" and "FALSE" may mean something to the PLC programmer which is not obvious to the end-user. These should be handled with care and not left up to interpretation.
I strongly disagree, this is a minority of cases. In these cases they can be handled with care and customized, like with the power "On" vs "Off" booleans, which still make sense to the user if left as power "TRUE" vs "FALSE". The strong majority of cases are stuff like enable, reset, execute, done, and busy bits where the end user is getting very confused by "Zero" vs "One" in the typhos screens from my experience sitting in the control room.
If we do buy the argument that these sorts of things should be set with care, we have to make things like ONAM
and ZNAM
mandatory includes in the pragma with an exception raised if missing. This would be preferable to letting a PV through that needs to be revised later because it has been given a meaningless default value. I'm currently sitting in a position where I have to track down all these PVs and I would have strongly preferred either a sensible default or a "hey, fix these things now, ok?" message. I'm also looking ahead and not pleased with the prospect of forgetting to set these manually, or with having to teach every new user of pytmc that this is a thing they need to do.
PRs are welcome.
I made two PRs for my two suggestions. There isn't much to implement here, it's just discussing what we want to do...
Currently the defaults for
BinaryRecordPackage
are: https://github.com/slaclab/pytmc/blob/7290ba67a91fca2970a4311cf3a165ab7c5cc696/pytmc/record.py#L471This is unintuitive because the only TwinCAT type that maps to
BinaryRecordPackage
isBOOL
: https://github.com/slaclab/pytmc/blob/7290ba67a91fca2970a4311cf3a165ab7c5cc696/pytmc/record.py#L637Since
BOOL
is the only TwinCAT type that usesBinaryRecordPackage
and becauseBOOL
shows up asTRUE
orFALSE
while online in TwinCAT, the default should be changed toTRUE
andFALSE
, since the usage is overwhelmingly as a boolean flag rather than as a number. In fact, in our projects there isn't a single case where the default values are used intentionally.If there is some disagreement here then I will soon go through all of my TwinCAT code and copy/paste the same field lines to all of my
BOOL
pragmas.