Please help me to rewrite the code

In rung 6 take out everything except the timer .DN bit. You've already got it doing the same thing for you in rung 4.

Rung 7 looks rather suspicious to me, is it your intent to only check to see if conveyor 1 has been started, and not actually running?
 
IMHO and experience, the original code is more mainstream with respect to starting and stopping conveyor motors.

That said, there seems to be redundancy with the timers and stop/start logic. For example, the stop bits may not need to be on both the timer and OTE instructions. I cannot say for sure without knowing whether the timer bits are used elsewhere in the program.

My advice would be to consider the purpose of the timer, and limit its precondition to just those aspects that you want to delay. For example, is it just sensor response time ("de-bounce"), or does it include startup/run time of a downstream conveyor? Then use that .DN bit, latched and unlatched as necessary, to energize the OTE. Again, in this example, the stop bit would unlatch the OTE, but not be part of the timer, unless the timer is used elsewhere.
Mispeld, These timers are just for sensor response time because this is inspection line and door will be moving on the conveyors, I want to avoid the situation where guy comes to conveyor and lift up the door for a sec to have a look so this timer will avoid that debounce time.

Thanks,
 
I can suggest a couple of things. You might want to add descriptions to some of those bits. For instance, "Line_2_Running_Bit", is this monitoring the MCR for the line? Or is it an Auto Mode flag? Try to make it so that people could understand exactly what each instruction is doing without seeing the machine the run.

The code itself isn't overly complex. If you clarify what everything is doing I think you'll be ok. Don't be afraid to document too much. Use rung comments and descriptions whenever you can.

yes line 2 running bit check the state of MCR and thank you for the advice, I am going to comment each rung and instructions so this will help someone in future.

Thank you again,
 
thanks everyone for comment and I am gong to stick with the same code but try to reduce the code and also I will put comments and description in my PLC code.

Thank you for the input!
 
Having a hard time understanding what you want. You said you have 3 motors and 3 prox's. What is this part present for the conveyor? Is the prox the part present or are you trying to use a memory bit?
 
Let me start by saying congrats on completing you first program. I’m assuming this thread is about the conveyor you posted about in the My first PLC project finished! thread.

The way I would have tackled the program will take more lines of code than the 5 you are showing. Your code does take a part placed on the conveyor and moves it to the first open spot on the conveyor. But in watching the YouTube video you posted I see some problems. As you are eluding to in this post, the part movement is inefficient. With your code the part stops at each conveyor or what I would call a station before it moves to the next station even though there is no part at the next station. What I think you’re looking to accomplish is when the conveyor is empty and you place a part on it, the part should be moved to the front of the conveyor without stopping. There are some other issues as well, what happens if a motor fails to run (tripped overload), what happens if a part is taken off the conveyor, what happens if a motor is running but a part doesn’t move? What if the conveyor had 6 stations instead of the 3? If you start adding to your program to account for these what if’s, you might end up with a mess. The issue I see is that your program isn’t broken down into logical blocks, it lacks structure or architecture.

First, I would change your naming convention. The entire piece of equipment is the ‘conveyor’, so I wouldn’t have named the parts of the ‘conveyor’ (like the motors and sensors) conveyor[1], conveyor[2], conveyor[3]. This causes some confusion, does this program run more than one ‘conveyor’? Your comment for rung 4 denotes a better name for these parts of the ‘conveyor’, that being Section. Personally I would call these parts of the ‘conveyor’ Station1, Station2, Station3, but even Section1, Section2, and Section3 is better name than conveyor[1], conveyor[2], and conveyor[3], plus it’s easier to read. You might also consider using Camel Case when naming your symbols, some designers limit the amount of characters you can use for a name and using Camel Case will allow you to get rid of the dashes and still allow your names to be readable, i.e. Station1MotorControl. Also, don’t put the word ‘bit’ in your names (line_2_running_bit), we already know it’s a bit ;). When you name symbols, determine if the symbol is a command or status and name it accordingly. I’m thinking Line2Running is a command, so it should be called Line2Run. Your Motor_control would be called Run because it’s a command, a signal back from the motor would be called Running. You have a Start_sensor and Stop_sensor, but is that what they really are? Are they the InfeedSensor and OutfeedSensor? Your Start_timer, we already know it’s a timer, what’s its purpose? Would a better name be StartDelay or RunDelay?

Order of the logic. When I look at the five rungs you posted I see one thing common between all of them, that being line_2_running_bit. But where that bit is placed on each of the rungs it’s hard to see that it is common. If you place the line_2_running_bit bit (that sounds redundant now doesn’t it ;)) first in the five rungs it makes it easier to see why an entire section of code isn’t running. This will make it easier to troubleshoot.

For the logical blocks, the program structure, I would have done this. There is a Conveyor object and Station objects. The Conveyor object controls the state of the conveyor. It knows the push-button states, controls the indicators, e-stop logic is probably done there as well. It determines if the conveyor should run or not. The Conveyor object tells the Station objects to run. The Station objects tell the Conveyor object if they are in alarm. The Station objects convey information to one another, am I Ok, can I receive a part, am I sending a part. The code for Station 1, 2, and 3 is the same except for mapping of information between the stations, this gives you code reuse. How a station determines how they run their motor, whether there is a part present or not, doesn’t matter to the other stations. The other stations only care about, can I send you a part, are you Ok, and here comes a part. So the logic for Station1 shouldn’t have Station2’s senor or motor status in it, it should only have Station2’s StatusOk and InfeedAvailable status in it. Coding to logical blocks lets you focus on smaller pieces for the program instead of the entire program at once. Notice your original post is concerned with how to control all three motors. What you need to think about is how to control each individual motor. A Station is only concerned with the station before it and after it. So in actuality you only need to think about how to control and individual station. From there you organize your data into logical blocks of memory, so you might use one word for the status of the station and another word for the alarms of the station.

Capture.jpg
 
Let me start by saying congrats on completing you first program. I’m assuming this thread is about the conveyor you posted about in the My first PLC project finished! thread.

The way I would have tackled the program will take more lines of code than the 5 you are showing. Your code does take a part placed on the conveyor and moves it to the first open spot on the conveyor. But in watching the YouTube video you posted I see some problems. As you are eluding to in this post, the part movement is inefficient. With your code the part stops at each conveyor or what I would call a station before it moves to the next station even though there is no part at the next station. What I think you’re looking to accomplish is when the conveyor is empty and you place a part on it, the part should be moved to the front of the conveyor without stopping. There are some other issues as well, what happens if a motor fails to run (tripped overload), what happens if a part is taken off the conveyor, what happens if a motor is running but a part doesn’t move? What if the conveyor had 6 stations instead of the 3? If you start adding to your program to account for these what if’s, you might end up with a mess. The issue I see is that your program isn’t broken down into logical blocks, it lacks structure or architecture.

First, I would change your naming convention. The entire piece of equipment is the ‘conveyor’, so I wouldn’t have named the parts of the ‘conveyor’ (like the motors and sensors) conveyor[1], conveyor[2], conveyor[3]. This causes some confusion, does this program run more than one ‘conveyor’? Your comment for rung 4 denotes a better name for these parts of the ‘conveyor’, that being Section. Personally I would call these parts of the ‘conveyor’ Station1, Station2, Station3, but even Section1, Section2, and Section3 is better name than conveyor[1], conveyor[2], and conveyor[3], plus it’s easier to read. You might also consider using Camel Case when naming your symbols, some designers limit the amount of characters you can use for a name and using Camel Case will allow you to get rid of the dashes and still allow your names to be readable, i.e. Station1MotorControl. Also, don’t put the word ‘bit’ in your names (line_2_running_bit), we already know it’s a bit ;). When you name symbols, determine if the symbol is a command or status and name it accordingly. I’m thinking Line2Running is a command, so it should be called Line2Run. Your Motor_control would be called Run because it’s a command, a signal back from the motor would be called Running. You have a Start_sensor and Stop_sensor, but is that what they really are? Are they the InfeedSensor and OutfeedSensor? Your Start_timer, we already know it’s a timer, what’s its purpose? Would a better name be StartDelay or RunDelay?

Order of the logic. When I look at the five rungs you posted I see one thing common between all of them, that being line_2_running_bit. But where that bit is placed on each of the rungs it’s hard to see that it is common. If you place the line_2_running_bit bit (that sounds redundant now doesn’t it ;)) first in the five rungs it makes it easier to see why an entire section of code isn’t running. This will make it easier to troubleshoot.

For the logical blocks, the program structure, I would have done this. There is a Conveyor object and Station objects. The Conveyor object controls the state of the conveyor. It knows the push-button states, controls the indicators, e-stop logic is probably done there as well. It determines if the conveyor should run or not. The Conveyor object tells the Station objects to run. The Station objects tell the Conveyor object if they are in alarm. The Station objects convey information to one another, am I Ok, can I receive a part, am I sending a part. The code for Station 1, 2, and 3 is the same except for mapping of information between the stations, this gives you code reuse. How a station determines how they run their motor, whether there is a part present or not, doesn’t matter to the other stations. The other stations only care about, can I send you a part, are you Ok, and here comes a part. So the logic for Station1 shouldn’t have Station2’s senor or motor status in it, it should only have Station2’s StatusOk and InfeedAvailable status in it. Coding to logical blocks lets you focus on smaller pieces for the program instead of the entire program at once. Notice your original post is concerned with how to control all three motors. What you need to think about is how to control each individual motor. A Station is only concerned with the station before it and after it. So in actuality you only need to think about how to control and individual station. From there you organize your data into logical blocks of memory, so you might use one word for the status of the station and another word for the alarms of the station.

Tark,
First of all, I would like to thank you for taking your time out and gave your feedback on my thread. I really appreciate that!

I didn't post the complete code in this thread but I tried to show you 5 lines of code and how can I rewrite those 5 lines using a move instructions to look more professional code.

In reality, There are 6 conveyors in total and I created UDT called "conveyor_control" and there is 6 parameter of this UDT.
1. Start sensor
2. Stop sensor
3. Start timer
4. Stop timer
5. motor run
6. Unmatched condition timer

Then I created conveyor array and used "conveyor_control" data type for that array and then I get conveyor[0] to Conveyor [7]

Now for conveyor 1, I am using followings tags

conveyor0.start_sensor
conveyor0.stop_sensor
conveyor0.start_timer
conveyor0.stop_timer
conveyor0.motor_run
conveyor0.Unmatched condition timer, this timer will turn ON when part present on conveyor 1 and no part present on conveyor two and conveyor 1 will stop when part reached at conveyor 2 but if someone picks up the part while traveling in between conveyor, so my part will never reach to conveyor 2 and my motor for conveyor 1 will never stop and this timer will stop the motor after 20 sec ( if average time to reach part is 10 seconds and motor continue to run )

But I agree with your points, I should use name more realistic like instead of using running bit, I will change to line run and so on. Thank you for the advice :)

I also liked your program strcutre and i will try to follow some of it and will implement some of my ideas but I would like to use AOI (add on instrcution) because it will make easy to troubhlshoot and change the code. I dont know how to use AOI but I am learning it and will change that code and update you guys!

Please let me know if you want to see my code? I will be at work tommorow and will send you my .ACD file.

Thank you again for your time and advice and sorry if i missed something!

Best,
 
It doesn't matter what code looks like, as long as it works (more or less)

If you want to help the next guy then comments, comments & more comments.
 
Be cautious with how you use AOIs. They are mainly used to protect parts of the code that you don't want people to see or change and they can't be edited online, so the code itself needs to be absolutely mistake proof.
 
Be cautious with how you use AOIs. They are mainly used to protect parts of the code that you don't want people to see or change and they can't be edited online, so the code itself needs to be absolutely mistake proof.

I am going to stick with this code but I am just trying to learn AOI and I think this is the best way I can learn AOI in real project.

AOI for this project is for learning purposes and I will get back to my original code.

Thank you for giving me heads up on AOI.

Best,
 
Don’t strive to make your code ‘look more professional’, strive to make good code. Good code is code that is understandable and maintainable.

Choose good names when naming your elements, this helps to make your code self-documenting.

Use rung comments. I comment every rung. Each comment has at least the name of the rung, and then if the intent of the rung isn’t clear I’ll explain its purpose. In your image, rung 6, I would have a comment like “Conveyor 1 Motor Control”. Rung 5 might be called “Conveyor 2 Start Delay.” /n “Used to delay motor startup to allow … “. Generally rung comments should explain why, not how. In the images you posted I can clearly see how your rungs are doing what they do, but I don’t know why they are doing it. If a rung is doing some masking, bit shifting, some type of advanced stuff, then I add how to the comment.

As far as the logical blocks go, I was showing a different way to break down your program. Where you focus on a small piece, you determine what this piece of code needs to do, what it needs to know, and what it needs to make known. I’m thinking your UDT could use some status information, such as PartPresent, PartInbound, PartOutbound. With that status information then you could add alarms to your UDT like PartFailedToEnterAlarm, PartRemovedAlarm, and PartFailedToExitAlarm. In the image I posted, if Station 1 told Station 2 PartInbound, this would tell Station 2 it should run its motor to receive the part. With that being the case would Station 2 need a start sensor? Also, couldn’t Station 1 turn off its motor once the part had passed its stop sensor, wouldn’t this signify that Station 1 no longer has the part? And, if Station 2 failed to detect an inbound part then it would turn on its PartFailedToEnterAlarm which would turn off its motor. I think that would take care of the logic you’re trying to figure out with the Unmatched condition timer.
 

Similar Topics

Please help me, I have solve many week but still not solve it. I found trouble of factory talk studio when I set tag by browse address of OPC...
Replies
0
Views
82
Hello Everyone, i Have im my Industry a Endress & Hauser Promag400 this has a screen that constantly have that error, it says to wait, somebody...
Replies
2
Views
428
After replacing the 70 with the 525, the PLC can read from the drive and recognizes it as online, but no commands are being listened to. PLC is...
Replies
1
Views
495
To quickly test a plc output which is wired to a relay do I dob a cable between the output and 24vdc+ source I.e something with 24vdc+ live such...
Replies
6
Views
637
"Hello, I am a beginner learning about PLC. Could you please give me some advice? I want to write PLC instructions as follows: When the sensor...
Replies
18
Views
3,239
Back
Top Bottom