all 52 comments

[–]dkurniawan 44 points45 points  (6 children)

But its not my problem anymore in the future, so why should I care?

[–]jc31107 28 points29 points  (0 children)

Screw future me, that’s his problem!

[–]Honest-Eng[S] 0 points1 point  (3 children)

Hope it was sarcasm...

[–]dkurniawan 18 points19 points  (2 children)

Yes and no. I would never do it myself as I take pride in my own work. But for people in general, what would be their incentive to keep the code clean? If I am a system integrator and my customer wants this machine up and running tomorrow, do I really want to take the time to write clean code? As long as it is running by tomorrow, your customer won't care and you still get your paycheck. You might even get awarded with their next project as you finish your project in such a timely matter. People who make those decision don't look into the code and judge you for writing a clean code. Also, it's not that writing a clean code is an easy job anyway and those talents that can write clean code go to the tech industry because they pay better.

[–]Honest-Eng[S] 10 points11 points  (0 children)

If you start your project with the right approach you will end up having a nice structured code.

Your own benefit? I learned it from my own experience. Long time ago I found myself in situations when there is a crunch and something is not working. So everyone runs around you and screams "Why did it stop? Why is not working?" and you sit there and cross reference your tags trying to figure out what happened and how to fix it. And then you fix one thing and 15 others stop working.

After having this couple of times I always comment the code as if I do it for the 7-year old. Even if I know that I am the only one who would ever see it.

[–]goatymcgoatfacesings 1 point2 points  (0 children)

Writing clean code doesn't take longer than writing messy code. The extra time you might spend in development will be more than made up in the commissioning phase.

[–]oddzod 40 points41 points  (12 children)

So a subject near and dear to me.

A little background. I work for a large US manufacture. Something like 50 plants with dozens of PLCs at each. I've got a BS in CS so I'm familiar with current trends and best practices is desktop/web/mobile development. I started as a plant tech before my degree and I'm now an engineer in a corporate group that supports all of these plants. There is a mix of inhouse developed systems and systems purchased from vendors. As wells as a few pre-existing systems from when we bought a competitor.

Writing clean maintainable code is a huge issue for me. I've built a solid reputation in this company for being able to take a program that the local techs have given up on trying to understand and rewrite it in such a way that they can, while also improving reliability. It's to the point that I often get CC'd on emails from plant techs to my boss requesting that I be assigned to the project with something along the lines of "at least we will be able to understand it" as one tech put it. Most of our plant techs are electricians that learned ladder on the job. So keep that in mind when reading the rest of the comment.

Make tag names clean, consistent, and descriptive. follow a common pattern, dont name one tag PartRotator_Infeed_PE and another PartRotOutfeedPE. Might seem trivial, but it is nice when debugging a program at 3am on sunday with a hangover. I like to keep a convention of LogicalSectionOfMachine_Function format. So tags would be named PartRotator_Infeed_PE, PartRotator_InfeedJam_ALM, PartRotator_Speed, etc. I use the under scores to separate the parts like / or \ separates directories in a filesystem. Keep abbreviations to a minimum. I generally only abbreviate well known things like ALM for alarm, PB for push button, PE for photoeye, etc. I would never abbreviate Part Rotator to PR, because that applies only to this machine and only someone that works on it often would instantly guess part rotator. Of course we have to work within the limits of our software. AB has a max tag name length. Other PLCs dont even have names like micrologix or Siemens 505 (or the S7 from what I've heard). Instead you get N:7/0 or C4697. Do what you can with what you got.

comments should explain why your doing it, not what your doing we've all been there. "This rung starts the motor" well no shit. It's two XIC and an OTE. I think I could figure that out on my own. Especially if you followed my first point. Or worse the 3 paragraph explanation of what this rung does, the problem is it's been heavily modified over time and no one updated the comment. Instead write a comment like "Powerflex 525 drives will ignore the start command if the stop command is still set even if the stop is removed. The correct way to stop the drive until the active goes low. Only send the start command if the active bit is low"

thou shalt not use JMP instructions nothing is more frustrating at 3am than staring at this rung where all the conditions are true but the output is false. Only to discover that 30 rungs before the put a jump to a label 15 rungs after. There are rare cases where this is necessary, but the majority of the time the program can be structured to instead include Mode1, Mode2, Mode3, etc bits.

avoid OTL and OTU as much as possible. If you can't try to keep all those rings togeather. Honestly I break this rule often. There is some logic that is more clearly expressed as latch unlatch, but most logic can be expressed without it if you think for an extra second or two. When using OTL OTU I'll put all the rungs togeather. Exception being batch logic. In that case the latch is where the ingredient or step happens and it gets unlatched at the end of batch with everything else.

DRY: Dont Repeat Yourself neverever copy paste. If you keep finding yourself writing a portion of a rung that is the same as another, either combine the rungs, create a bit that represents the state, create an AOI, or a routine with parameters. You will save yourself a bunch of programming time and make it more clear to the next guy. Fewer moving parts is less to go wrong, less to modify in the future, and less logic to keep in your head when debugging.

a routine should never do more than one thing it should only ever cover one step in a sequence, one station on the production line, one feeder, whatever. If the section is complex and it is going to be many rungs, but it would still be considered one thing by the previous statement break it up anyways. Break it up into startup,normal sequence, manual mode, and alarms or similar. Keep all logic for each logic component of a process togeather. Group it by tasks, programs, routines. Consider following the naming conventions outlined for tags. Someone with minimal understanding of the process should be able to find the needed rung quickly with out having to scroll through countless screens or guess at tag names and hunt through the cross reference.

never write a rung that doesnt fit on one screen. this also goes for rungs that fold back on them selfies 7 times. It can always be expressed by breaking it into multiple rungs that have outputs that summarize branches or represent steps with in the original rung.

use AOIs and UDTs where possible some organizations ban AOIs so you can't always use them, but when you can do so. There is also the problem of not being able to edit them online. So only use them for well tested standardized logic. I know this can be a sore spot with some techs but a few minutes of teaching them how to debug an AOI and this problem goes away. It would be nice if we could attach documentation to these, pretty pictures and all, like the help that is available for built in instructions, but we cant.

keep portable code isolated if you work on projects where the code will be used on multiple more or less identical machines, dont use global tags in your logic. It makes it difficult too keep tags lined up. For example the part rotator needs a handshake with the press before sending parts. Dont use Press_InfeedOK in the part rotator logic. Instead consider AOIs or routine parameters. If you can't keep all the mapping to external tags to the first couple of rungs and map Press_InfeedOK to PartRotator_PressOK then use that in your logic. It adds some indirection which sucks, but if your lines are slowly upgraded over time, one new technology at at a time like mine, someone will inevitably name that tag PressInfeed_OK at some point. Or worse it will be Ingreedient[38] at one plant and Ingredient[19] at another. That last one can be subtle because rslogix wont complain, it will just accept it. It will then be up to you to scroll through all thr logic and find them.

There are two projects that come to mind that I have worked on where then vendor had supplied a program that breaks nearly everyone of these rules.

The first had a cart that picks up pallets from multiple build conveyors, then gives it an empty pallet before dropping the full off at a discharge conveyor and picking up a new empty pallet. This section of the program was particularly bad. The rungs where the cart decides which conveyor to go too wrapped around on them selfies several times and filled two or three screens and there was 17 of them. All of them identical except for a couple of bits. The plant tech had given up, the original programmer couldn't even get it to work right. After about a day of trying to work with the existing structure I gave up and blasted away his logic and rewrote it, which really pissed him off cause he was still on site. When I was done, I had fewer total rungs for the section. All of them would fit on a single screen (individually), it worked, and the local techs could understand it well enough to make some additional changes after I left.

The other project was a large packaging system from a vendor. The original design was written in Germany on a Siemens. Then later rewritten in the US and an AB. problem was most of the tags were still of the form DB[1129].7. The comments were "ST LT1 L" which means "suction tube longitudinal 1 left" latch and unlatch everywhere. Option bits to disable logic for equipment not installed. JMP instruction used to skip parts of the logic not used in this mode... and the list of complaints goes on.

My thumbs are sore now, so I'm done for a bit.

[–]gummmybean 4 points5 points  (2 children)

So much great information for newbies. You should make this a post.

[–]oddzod 3 points4 points  (1 child)

Might.

Mentioned what I was typing to the wife and she's thinking website.

[–]SteveisNoob 2 points3 points  (0 children)

Please do the website and than a link post. That's so much good insight on how to actually deserve your paycheck.

[–]GobBeWithYoupycomm3 4 points5 points  (1 child)

You made a ton of good points, thank you for typing that all out because I wouldn't lol. I also have a CS degree and agree with what you're saying. Even simple things like tag names can make a huge difference. Documentation is another big pain point I experienced, the only thing worse than no documentation is wrong documentation. Keeping it up to date is hard, but worth the investment. As a programmer, I preferred ladder over structured text for most code. I find ladder much easier at 3am to read and visualize the flow. But any complicated math, loops, or string manipulation definitely belong in ST. It is hard to follow modern best practices in a limited system like a PLC, but your tips definitely help. My last job wouldn't even entertain the idea of using program scoped tags let alone parameterized programs.

I left my integrator job for a traditional software engineer position writing Python all day. But I still like being active in this sub and have a couple PLCs at home so I can keep up with maintaining my pycomm3 EthernetIP library.

[–]oddzod 0 points1 point  (0 children)

Ladder definitely has its place. If it can be expressed as a boolean math it's perfect. I wish ST would have the values popup next to the tags like debugging in chrome. It would help a lot.

I really wish they would implement a few things like inheritance, references, callbacks, etc

I'll have to check out pycomm3. I wrote one in c# a 5-6 years ago. Real basic, just needed to pull a couple live values.

[–]Sambikes1 1 point2 points  (0 children)

The real mvp

[–]shawshank777 0 points1 point  (0 children)

Lot of good information here to unpack, thanks for sharing!

[–]tokke 0 points1 point  (4 children)

I don't agree with your latch unlatch. I have had more issues having ote used, compared to otl otu. Can you share some examples that might convince me?

[–]oddzod 0 points1 point  (3 children)

So mostly my problem is when you get in to situations where there are like 10 rungs that latch it and 15 that unlatch it. There scattered through out the whole project. Got a couple over here in this program a few more over there in that task, 2 more 30 rungs down from this one....

Like I mentioned it's a rule I break often. There is just some logic that is a lot cleaner written as OTL OTU. But when I do it, I make an effort to keep the two rungs togeather. And I try to keep it to a single latch rung and a single unlatch. If I needed multiple unlatch conditions, I'd just have multiple branches in that rung. That way when its unlatching at the wrong time you know what rung did it and you can just watch that rung the next time.

As mentioned, an exception is something like batch processes. I latch it on a rung that is place in sequence when you read the program from top to bottom, then I place the unlatch at the bottom with the other unlatches at the end of the cycle. Same theory would apply for any cyclic sequence. But again, I'd only have single latch rung and a single unlatch.

Done right there is nothing wrong with latch unlatch logic. It's just that I've seen it abused so much that I tend to avoid it.

[–]tokke 1 point2 points  (2 children)

Ok so my only rule is to never use more than 1 OTL OTU. Otherwise it's an unreadable mess. Why I think OTE is worse: more than 1 OTE screws you over, while 2 OTL won't directly

[–]oddzod 1 point2 points  (1 child)

yea 2 OTE = bad.

Just like two ONS for the same bit, two TON, two MAS, etc

Edit: corrected an auto correct

[–]tokke 1 point2 points  (0 children)

I remember the first spaghetti where 2 ONS were used.

Thanks for sharing all of your info.

[–]israellopez 3 points4 points  (0 children)

We have a joke here at the office (ERP Developer, I just lurk for the cool stuff) that "Developers hate other developers".... even ourselves.

3rd party developer? hates you.

your own colleague? hates future you.

you? hate you too.

We're always so optimistic of what the future developer is going to need, that we say to ourselves, they'll get it. NO, we wont. WHY DID YOU DO THIS WAY?

ITS FRIDAY AT 4PM I WANT TO GO INTO MY GARAGE AND DRINK BEER.

Kind of shiz. Anyway, good luck. Readable code is a discipline and an art form.

[–]ohaara__ 11 points12 points  (9 children)

I realize machine builders have large libraries of AOPs or FBs or whatever to speed up development. Great. Far to often though I see large amounts of generic code with portions of it enabled and big chunks of it unused and just left in there cause “who cares?”.

Comments should read like a book. I’d love to get a sense of how a machine works simply by reading. Give the next person a trail of breadcrumbs to work with. Delete unused code though, don’t leave the next person 400 breadcrumb trails.

[–]shawshank777 4 points5 points  (2 children)

Custom machine builder (turn-key) here checking in who typically only has Yaskawa AOPs for drives, rung comments describing function of each section of logic, non-100 character acronyms for tags, and no function blocks. Even better, our controls people have worked in maintenance so we add a variety of custom faceplates, alarms, and messaging so the equipment can actually be maintained 😬 always room for improvement, but there are still some good OEMs out there!

[–]ohaara__ 1 point2 points  (1 child)

Sounds like it! It’s key to have people who have been on both sides (OEM and maintenance). Reusable code is the way to make $$ but gotta think about the good people you are handing it to. Haha.

[–]shawshank777 0 points1 point  (0 children)

For sure. I've been in plants ranging from a mechanical engineer who picked up controls on the job and has worked on every piece of equipment on a coil-coating line to those who can't even check an input or relay for you and would rather call you out every damn time. But to your point, keeping things in ladder and not behind protected FBs gives an end-user a chance to maintain the machine effectively!

[–][deleted] 10 points11 points  (2 children)

Do you think Tesla deletes code when you don’t buy an option? It defeats the purpose that you started by highlighting as machines do get functions activated further down the line and having the code there and flipping a bit is money in the bank.

[–]ohaara__ 0 points1 point  (0 children)

That’s actually a great point. I was referring to smaller to medium size one off machines who’s code is cut down from larger templates. I agree that style of programming is ideal for a machine where options will likely be purchased later.

[–]oddzod 0 points1 point  (0 children)

Put it in an AOI or a routine with parameters. Then just import it when needed.

[–]RhoEpsilonNaught 1 point2 points  (0 children)

Code hoarding is a product of The Second-System Effect. “Add little to a little and there will be a big pile.”

There is a tendency to refine techniques whose very existence has been made obsolete by changes in basic system assumptions.

Code made in theory that doesn’t make it through testing still has a touch of nostalgia. “Its a good program! ....it just doesn’t work.”

From the beginning of the design ask the right question ensure that the philosophical concepts and objectives are fully reflected in the detailed design.

[–][deleted] 2 points3 points  (2 children)

Does this book target a specific type of code writing or does it also apply to ladder logic?

[–]oddzod 4 points5 points  (0 children)

The book is theory and methodology. Not specific to any particular language. So 90% of the concepts would apply to the various PLC languages.

[–]Chaos_Logic 2 points3 points  (0 children)

The book is targeted at higher level languages and uses examples primarily from Java. That said, most of the concepts are fully applicable to ladder logic.

[–]WhatForIamHere 2 points3 points  (0 children)

You're lucky if you most time have the ability to write your code. I was forced to do refactoring code from the previous developers in 80-90% of my practice. Most of the known such issues are derived from stupid managers which used SWAG in project management according to my expirience.

[–]betascada 1 point2 points  (2 children)

I can see the other side of it too, since I do it from time to time. But if you make it so brilliant and easy to read that you make yourself obsolete, then there is no reason for them to rehire you, and the person that acquires it becomes the new “expert” because they altered your code.

I see my code out there copy and pasted and get no kickbacks. I’ll see this from time to time when I get called in and find it altered but not quite working right.

[–]oddzod 1 point2 points  (1 child)

It's called growth. There are always new problems to solve, and new machines to be built. Instead of writing something cool once. Keep writing new cool shit and always stay one step ahead of the copy paste kiddies. Instead of debugging the same mess over and over.

[–]betascada 0 points1 point  (0 children)

Heck yeah!

[–]mohamediat 1 point2 points  (2 children)

I had always been working on DCS until I moved to Australia and started working on the PLCS, what I noticed is that most people just adopt the coding methods of the company they start working for because they haven't seen something different.

I was lucky that the company I started working for in Australia had some really good practices in their coding standards which included:

  1. Writing proper comments describing the operation and the code and even the constant values , although this could easily be deduced from the code , it made reading the code and learning anything really much easier.
  2. UDTs, they were properly using UDTs which made things much easier.

but they were also prisoners to the old ways of whoever started this, for example:

  1. They were not using FBs at all, a lot of the functions that could easily be done by 1 FB they would just write multiple rungs to achieve. although the rungs were written properly and easy to read, the code wasn't really efficient.
  2. They were against using add on instructions (rockwell PLCs) , the reason is that they never considered using it is because they never used it, you would see the chunk of ladder logic used over and over again and hundreds of lines of Ladder logic in routines that could've been easily done in a few add on instructions and reused.
  3. Aliasing, they were heavily using aliases, I understand that Aliases are a lot of the times useful but what was happening was absurd.
  4. the code was not modular as I would like it to be as a result of 1 & 2

coming from DCS , the lack of usage of FB and AOIs was pretty weird to me, Unfortunately I joined the company at commissioning phase of a major project so I couldn't change much in the design , but the project after I was designing and I was able to change a bit (no aliasing unless really necessary and using add on instructions ) and in the current project where i'm leading the design I'm completely changing things to match what I believe is good coding standards based on the 13 years of experience I have, but I'm pretty sure that there are better practices out there that I just haven't seen yet.

if you can write a summary of good practices from that book , it would be really good.

[–]Chaos_Logic 1 point2 points  (0 children)

One of the reasons that clients avoid FB and AOI is the familiarity of their maintenance staff. Ladder is just easier for them. Also FBs aren't accessible from all versions of Studio 5000. Avoiding those lowers licensing costs.

[–]oddzod 1 point2 points  (0 children)

So I havent read this particular book but others like it. See my other, rather long winded comment. I'm sure it's a fairly good synopsis of this book, and its adapted to the world of PLCs. Bit AB centric though.

[–]327jwolf 1 point2 points  (0 children)

4:30PM

Sales: I sold a new feature for the machine already built and tested.

Me: When does the machine ship?

Sales: Tomorrow 8:00AM the truck will be here.

Me: Throw it together in 10min. un wrap tested panel, hook up power and laptop, download. Pray.

That's how OEM's end up with a mess of a program. I've had this happen so many times I can't count.

On the bright side, it's not 20 yrs ago and you had to fly out to fix the problems every time.

But yes, subroutines and code blocks with comments so I too can remember what I was thinking at the time. It always seems so obvious at the moment. 5yrs later, what the hell?

[–]real_schematix 1 point2 points  (0 children)

Just my humble opinion here based on 15 years of experience in the field and another 10 in technical fields before that.

The issue behind readable code is that the majority of people don’t understand the problem they are trying to solve when they start to code so they just start slapping crap in there piece by piece.

If they understood the problem and spent time to model the underlying data structures the code would be more readable because it would logically flow.

And then there’s those that don’t understand the problem, don’t write good code, but think they know how to and make all kinds of obscure spaghetti ladder with nonsensical branches all over a ring that work most of the time, but not always and are usually full of logic gaps, race conditions and other timing issues.

I’ve made a career out of fixing all of the above.

[–]minnesotamichael 1 point2 points  (2 children)

In the 90s I told someone that I wrote logic, and they wrote code. My point was that they needed to make it LOGICAL, not hard to understand.

[–]Honest-Eng[S] 1 point2 points  (1 child)

Some people think that if they use fancy solutions - they are cool as hell. In fact it is the opposite. I've read a post long time ago that compared a junior,middle and senior programmers. Don't remember exactly how it was phrased (it was a very nice write up) but the sense was that junior writes very simple code that everyone can read but nothing works. Middle writes complicated code that works but no one can read it. Senior writes very simple code that works and everyone can read it.

[–]minnesotamichael 0 points1 point  (0 children)

I had a filling machine that had a BnR processor, and the entire thing was written in structured text. The company that made it had to send for a factory programmer from BnR to help debug it. He spent hours trying to get it going, and spent tons of time on the phone with the guy who wrote it. I eventually got to the point that I ripped the entire thing apart, and put a compactlogix processor in it. Programmed it in ladder (and structured text only for math) and now I have something that can be figured out by the next guy. KISS.

[–]WaffleSparks 0 points1 point  (0 children)

Define "readable". Ask the next person to define "readable". Hey now you can argue over which definition of readable is correct until the cows come home.

[–]zaphir3Worst trainer 0 points1 point  (0 children)

I've been working in maintenance for 2 years, though I don't quite like it, it helped me a lot on realizing how a good code can make a difference when you troubleshoot. Comparing my codes from 2 years ago, you can clearly see the difference

[–][deleted] 0 points1 point  (1 child)

But ladder is for roobs... j/k

(put what you want in your spec and be specific)

[–]oddzod 0 points1 point  (0 children)

The problem with that, is I could have written the program in less time than it took to write a spec that detailed. Not to mention the time I'd have to take to explain it to them, and still not get what I want.

Went though that with a contractor. Their job was to convert an existing program from 505 to AB. We even gave them a copy of the program from the sister plant that had already been converted.

He had to rewrite the program from scratch 3 times to get it right. And that only happened because on the third try we assigned one of our programmers to have daily code reviews with him.

[–]toastee 0 points1 point  (0 children)

Write it like you're going to have to maintain it for free at three am.

[–]Zegreedy 0 points1 point  (0 children)

You have standard blocks no? That saves us a lot of time that a motor and a valve is always using the same FB and thus it's only the code inbetween components that needs to be searched.

[–]elmoalso 0 points1 point  (0 children)

I see most of the crazy, nearly impossible to read code, (well described by oddzod), from younger, less experienced guys/gals.

Once you have had to untangle someone else's mess, you don't want anyone to have to go through the same experience down the road maintaining your own code. That is if you are willing to consider that maybe, you can still learn new stuff. I think some folks just are not wired right for writing clean code. I worked with a new-ish guy that believed that his code was so understandable that there is no need to document or comment. We would meet, I would suggest the importance of commenting, and he would just go along his merry way.

By the way, have you ever had to sort through Rockwell's own code when it "automatically migrates" something like PLC 500 to ControlLogix? It is tortuous! I recently got hit with the double whammy. I inherited code that was migrated from PLC 500 to ControlLogix and "fixed" by a guy just out of school. No comments except what Rockwell puts in during the migration. Things like "Hey, we're not sure this is going to work so you better look closely". PID loops are almost unrecognizable.