Ok already



  • At the moment I am working at a client site commissioning what is basically a media converter (with cache) for a packet switched network [1]. Being at site is what is preventing me (and probably the rest of the team) from committing heinous crimes against the people who wrote the code we are trying to get working. While there have been numerous WTFs the following just irked me today.

    One of the systems I am working with is a Safety PLC. This is a type of programmable logic controller that has additional circuitry built into the I/O cards for doing hardware/electrical verification of the switches and sensors that are wired in. For example you could have two independent physical switches attached to the same device and the Safety PLC would automatically verify that they both changed state at the same time. This all and good (except for the PLC being of a certain German brand), but the use of these status values is where the WTF comes in.

    In the system I am working on there are many doors that open and close that are monitored by the safety PLC that use dual switches in the manner described above. And the people I am cursing and swearing at have have created things like (for example) a Door_Open variable that combines the state of the two switches and tells you that the door is open. They also created the Door_OK variables, which you would think are related to the quality state of the switches. But .. nooooooo .. thats the WTF. In this system Door_Open means the automatic processes have to take care, while Door_OK actually means that the door is closed and things can happen at will. Scatter a couple of dozen things like this through a code base that makes spaghetti code look like a viable alternative and you have a strong justification for using a clue by 4.

    [1] One side of the media converter interfaces to a ship, the other side interfaces to trucks, the cache is maintained by two automatic stacking cranes that run on rails and the packets are shipping containers that can weigh upwards of 40t.



  • @OzPeter said:

    But .. nooooooo .. thats the WTF. In this system Door_Open means the automatic processes have to take care, while Door_OK actually means that the door is closed and things can happen at will. Scatter a couple of dozen things like this through a code base that makes spaghetti code look like a viable alternative and you have a strong justification for using a clue by 4.

    I was with you right up to this bit, which makes no sense.



    You've used the definite article to talk about some "automatic processes" which you have not previously introduced to the story. Given that ambiguity surrounding the aforementioned (or should I say, afore-unmentioned) automatic processes, it is difficult to understand what is meant by them taking care.



    "Things can happen at will". What things? What happens? Whose will?



    I kinda want to know, because I do like the stack-of-shipping-containers-moved-by-cranes/data-cache analogy.



  • Blame my lack of clarity on lack of sleep because we are in crunch mode.

    Picture it this way .. I'm trying to debug code where the door being open (Door_Open) causes one code path to be followed, and that the door being closed causes another code path to be followed. I'm working in a system that maintains the quality of the its inputs .. thus Door_Open will have a quality associated with it. I'm staring at code that uses Door_Ok and I am wondering why the hell the quality status is being used in this manner .. it just doesn't make sense. Until I stare at it for long enough (and scribble enough) in order to understand that Door_OK is actually the boolean opposite of Door_Open.

    To top it off the code is being expressed in Instruction List format, and is like the following, only with more crap around it:

    
    ...
    
    LD Sw1
    ANDN Sw2
    ST Door_Open
    
    ...
    
    LDN Sw1
    AND Sw2
    ST Door_OK
    

  • ♿ (Parody)

    @OzPeter said:

    Until I stare at it for long enough (and scribble enough) in order to understand that Door_OK is actually the boolean opposite of Door_Open.

    So would a Door_Not_Found be more OK or Open?



  • @OzPeter said:

    Door_OK is actually the boolean opposite of Door_Open.
     

    Provided that the door is supposed to be closed, I don't see a problem.

     



  • I like the "packet switched network" analogy for shipping containers.  Very apt.  That was my previous life; networking that is, not shipping containers.  

    My current life is automation and have done programming and testing in Nuclear, Pharmaceutical, and Medical Device fields.  All I can say is many automation programmers do not understand good programming practices.  They usually have an engineering degree and learned the syntax for whatever system they program on and... viola, their programmers.   No theory on clean code required.  This has led to a litany of WTFs of spaghetti code, poorly named variables and sub-routines, and just bad code that just hangs when it reaches a state transition it didn't expect.  Even in the highly regulated environments that I work in, this is a significant problem.

    I have not actually seen Instruction List code before.  It just takes the problems you are describing and making them worse.  I have a new level of amazement in this day and age to use such a low-level language on a new build.  Most systems support several 61131 coding options.  Was IL a choice?  Do you have a decent debug environment to work in?

    Long answer short.  I feel your pain (queue visual of former US Pres Bill Clinton slowing shaking his fist, thumb up, to accentuate each word).



  • I must admit, that it's a pretty weird choice of name. I mean, OK clearly isn't the opposite of open.

    Presumably this is one of those case where the code isn't actually logically wrong, but has been named so badly that it makes it impossible to decipher what it does until you've decoded the weird substitution cypher where OK means Closed!



  • @cheapie said:

    @OzPeter said:

    Door_OK is actually the boolean opposite of Door_Open.
     

    Provided that the door is supposed to be closed, I don't see a problem.

     

     If I understand the OP correctly, the problem is that there are two variables storing one boolean datapoint, with one variable simply being the inverse of the other.

     



  • @OzPeter said:

    we are in crunch mode.

    Did one of your interfaces drop a packet?

    @OzPeter said:

    To top it off the code is being expressed in Instruction List format, and is like the following, only with more crap around it:

    ...
    
    LD Sw1
    ANDN Sw2
    ST Door_Open
    
    ...
    
    LDN Sw1
    AND Sw2
    ST Door_OK
    

    Assuming that Sw1 and Sw2 are Booleans, LDN is Load Not and ANDN is And Not, this code makes Door_Open and Door_OK obey the following truth table:

    Sw1Sw2Door_OpenDoor_OK
    falsefalsefalsefalse
    falsetruefalsetrue
    truefalsetruefalse
    truetruefalsefalse

    So Door_OK isn't the Boolean negation of Door_Open; if Sw1 and Sw2 are the same, both Door_OK and Door_Open will be false.

    Door_Closed would certainly have been a better name if the primary purpose of the code is to interrogate the state of the door - but if the door state is one of a number of things that have to be checked before it's known to be safe to do something, and the safe state of all those things is reflected in conditions with an _OK suffix, then Door_OK doesn't seem too bad.



  • @flabdablet said:

    Sw1Sw2Door_OpenDoor_OK
    falsefalsefalsefalse
    falsetruefalsetrue
    truefalsetruefalse
    truetruefalsefalse

    Having Sw1 and Sw2 imply an error state if they're equal makes sense from a hardware point of view. Assuming the interface pins are pulled to a common state when the inputs are disconnected, then the only way Sw1 and Sw2 can be different is if the cable connecting the interface to the switches is actually working; a disconnected or short-circuited cable would make both signals the same.



  • @OzPeter said:


    [1] One side of the media converter interfaces to a ship, the other side interfaces to trucks, the cache is maintained by two automatic stacking cranes that run on rails

    Ruby is TRWTF.




  • @DaarkWing said:

    and... viola, their programmers

    So much wrong with this!



  • @DaarkWing said:

    My current life is automation and have done programming and testing in Nuclear, Pharmaceutical, and Medical Device fields.  All I can say is many automation programmers do not understand good programming practices.  They usually have an engineering degree and learned the syntax for whatever system they program on and... viola, their programmers.   No theory on clean code required.  This has led to a litany of WTFs of spaghetti code, poorly named variables and sub-routines, and just bad code that just hangs when it reaches a state transition it didn't expect.  Even in the highly regulated environments that I work in, this is a significant problem.

    Just look at the tools such engineers use. LabVIEW, Ovation, this Instruction List language...it's no wonder bad code is the result. Spaghetti code is 6 times worse when it's done in a "graphical" language and there are boxes and lines everywhere on the screen and it looks like actual spaghetti.



  • @jello said:

    If I understand the OP correctly, the problem is that there are two variables storing one boolean datapoint, with one variable simply being the inverse of the other.

     

    The problem is that in a system that embodies quality status (ie . is the data OK), Door_OK is a very confusing name for the opposite state of Door_Open.



  • @DaarkWing said:

    ...viola, their programmers.
    Please tell me you did that on purpose.

     



  • @DaarkWing said:

    They usually have an engineering degree and learned the syntax for whatever system they program on and... wallah, there programmers.  
     

    FTFY.

     



  • @Zylon said:

    @DaarkWing said:

    ...viola, their programmers.
    Please tell me you did that on purpose.

    I feel incited to violins. What is the basses for such ignorance? He must be from the cello end of the gene pool.



  • @OzPeter said:

    At the moment I am working at a client site commissioning what is basically a media converter (with cache) for a packet switched network [1]. Being at site is what is preventing me (and probably the rest of the team) from committing heinous crimes against the people who wrote the code we are trying to get working. While there have been numerous WTFs the following just irked me today.

    One of the systems I am working with is a Safety PLC. This is a type of programmable logic controller that has additional circuitry built into the I/O cards for doing hardware/electrical verification of the switches and sensors that are wired in. For example you could have two independent physical switches attached to the same device and the Safety PLC would automatically verify that they both changed state at the same time. This all and good (except for the PLC being of a certain German brand), but the use of these status values is where the WTF comes in.

    In the system I am working on there are many doors that open and close that are monitored by the safety PLC that use dual switches in the manner described above. And the people I am cursing and swearing at have have created things like (for example) a Door_Open variable that combines the state of the two switches and tells you that the door is open. They also created the Door_OK variables, which you would think are related to the quality state of the switches. But .. nooooooo .. thats the WTF. In this system Door_Open means the automatic processes have to take care, while Door_OK actually means that the door is closed and things can happen at will. Scatter a couple of dozen things like this through a code base that makes spaghetti code look like a viable alternative and you have a strong justification for using a clue by 4.

    [1] One side of the media converter interfaces to a ship, the other side interfaces to trucks, the cache is maintained by two automatic stacking cranes that run on rails and the packets are shipping containers that can weigh upwards of 40t.
    I work on boring industrial stuff and I don't like how my coworkers use booleans.

    FTFY. Also I'm not sure about the industrial part, I can't tell if you actually move ships on a crane that got open doors and if so how exactly is the media involved. Also most networks nowadays are packet switched so I chalk that one up to name-dropping for the sake of name-dropping.



  • @gramie said:

    @DaarkWing said:

    and... viola, their programmers

    So much wrong with this!

    Seems legit to me:

    1. To viola one's programmers is to hit one's programmers with a viola. It doesn't count as assault in certain countries, and has similar thinking as the use of a clue-by-four. It's not a popular practice due to the expense of violas.
    2. One does not need to be pro-grammar.


  • @OzPeter said:


    [1] One side of the media converter interfaces to a ship, the other side interfaces to trucks, the cache is maintained by two automatic stacking cranes that run on rails and the packets are shipping containers that can weigh upwards of 40t.
     

    So, does this mean packet-loss is liable to result in stack-smashing?



  • @OzPeter said:


    LD Sw1
    ANDN Sw2
    ST Door_Open

    ...

    LDN Sw1
    AND Sw2
    ST Door_OK

    Another thing: it looks to me like there's a potential race condition in this design. Does the sensor interface include an explicitly controlled sampling latch? If it doesn't, then Sw1:Sw2 could go simultaneously from false:false to true:true (perhaps due to an intermittent cable fault) between LDN Sw1 and AND Sw2, setting Door_OK true when the door is not in fact known to be in a safe condition.



  • @Shoreline said:

    To viola one's programmers is to hit one's programmers with a viola. It doesn't count as assault in certain countries, and has similar thinking as the use of a clue-by-four.
    So that's what the news guys mean when they say someone was attacked with a blunt instrument.



  • @flabdablet said:

    Another thing: it looks to me like there's a potential race condition in this design. Does the sensor interface include an explicitly controlled sampling latch? If it doesn't, then Sw1:Sw2 could go simultaneously from false:false to true:true (perhaps due to an intermittent cable fault) between LDN Sw1 and AND Sw2, setting Door_OK true when the door is not in fact known to be in a safe condition.
    PLCs in general all run as a well defined cyclic process of

    1. Read the physical inputs and transfer their state to the input status tables
    2. Process the user program and write new outputs to the output status tables
    3. Transfer the output status tables to the physical outputs


    So the inputs are in a well defined state before the user program executes so race conditions like you suspected cannot exist. Typically you also have instructions that can force an I/O read or write cycle in the middle of the user program, but in my work I have rarely if ever seen that type of instruction used. For the fun of it the user program can also write to the input tables - which may seem weird but allows you to write modules for simulating physical inputs when you don't have them - ie when I am at my desk and I don't have a 40m high crane handy. Oh, and I am typically working with scan times of around 20ms. So the the complete read/execute/write cycle is performed every 20ms.



  • @OzPeter said:

    So the inputs are in a well defined state before the user program executes

    and it sounds like the user program doesn't actually have access to them, only to soft copies made in the same thread. Good.

    Do pairs of physical inputs belonging to a given sensor and its quality monitor, like the ones from whose state Sw1 and Sw2 are derived, get sampled several times and checked for consistency before being copied to the input status tables?



  • @OzPeter said:

    @flabdablet said:
    Another thing: it looks to me like there's a potential race condition in this design. Does the sensor interface include an explicitly controlled sampling latch? If it doesn't, then Sw1:Sw2 could go simultaneously from false:false to true:true (perhaps due to an intermittent cable fault) between LDN Sw1 and AND Sw2, setting Door_OK true when the door is not in fact known to be in a safe condition.
    PLCs in general all run as a well defined cyclic process of

    1. Read the physical inputs and transfer their state to the input status tables
    2. Process the user program and write new outputs to the output status tables
    3. Transfer the output status tables to the physical outputs


    So the inputs are in a well defined state before the user program executes so race conditions like you suspected cannot exist. Typically you also have instructions that can force an I/O read or write cycle in the middle of the user program, but in my work I have rarely if ever seen that type of instruction used. For the fun of it the user program can also write to the input tables - which may seem weird but allows you to write modules for simulating physical inputs when you don't have them - ie when I am at my desk and I don't have a 40m high crane handy. Oh, and I am typically working with scan times of around 20ms. So the the complete read/execute/write cycle is performed every 20ms.
    So...  you're working on an embedded device with no file system.

     


Log in to reply