Script not doing anything/ no errors

I have a change script enabled on 3 PLC tags (bool). On Value change, If value = true: I write PLC tag values to python variables, check to make sure all values are not 0 and check which position one device is in (need to write different values depending on its position), then run a named query logging this data in a DB.

I created a view to monitor the tag values to help monitor the process. I had this script working without a while loop but sometimes data points were missing so I added this while loop so that the data is not logged until all data comes in.

current script: (not working / no errors)

having trouble with the backquotes..working on it..

Please use the code formatting thing:
image

or enclose your code in triple backticks (you can even specify what language it is to enable syntax highlighting):

```python
code goes here
```

to keep your code formatted. As it is, it's unreadable.

1 Like

ill do that now, sorry

What I can tell you for sure, is that this script is bound to miss events.

Not only is it extremely inefficient to call read blocking multiple times, but you're doing it inside of a while loop.

On top of that currentValue.value will never change inside of the change event. That's bad, and is probably why your script "doesn't work but doesn't throw any errors" If I had to bet i would say it's stuck in an infinite loop.

Edit: I also, notice that you have tag names like 'Program:MainProgram', that is a practice that will cause you a headache at some point. Ignition tag names should be valid Jython Identifiers. Something like 'Program_MainProgram' is better, though I would tend to stay away from PLC location names as well. Name your tags for the values that they represent, there is no need for the tag names match PLC tag paths.

3 Likes

Here is the script, as posted.

def valueChanged(tag, tagPath, previousValue, currentValue, initialChange, missedEvents):
	if currentValue.value == True:
		date = system.tag.readBlocking("[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_DATE.value")[0].value
		time = system.tag.readBlocking("[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_TIME.value")[0].value
		job = system.tag.readBlocking("[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_BANO.value")[0].value
		roaster = 1
		item = system.tag.readBlocking("[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_1_.value")[0].value
		destcheck = system.tag.readBlocking("[~]GA_STATUS_40_.value")[0].value
		greenwt = system.tag.readBlocking("[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_3_.value")[0].value
		processed = 0
		destcheck = system.tag.readBlocking("[~]GA_STATUS_40_.value")[0].value
		dest = ''
		desttype = ''
		roastwt = 0
		while currentValue.value == True:
			roastwt = system.tag.readBlocking("[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_4_.value")[0].value
			if destcheck == 1:
				desttype = "Silo"
				dest = system.tag.readBlocking("[~]Program:MainProgram/RT_ROAST_AUTO_DEST.value")[0].value
				if dest != 0 and roastwt > 15:
					params = {'RoastDate': date, 'RoastTime': time, 'Job': job, 'Roaster': roaster, 'Item': item, 'Dest_Type': desttype, 'Dest': dest, 'Green_Wt': greenwt, 'Roast_Wt': roastwt, 'Processed': processed}
					system.db.runNamedQuery("Plant", "Insert Record to Roaster Production", params)
					break
			else:
				desttype = "Bag"
				dest = system.tag.readBlocking("[~]R1_Bag_Count")[0].value
				if dest != 0 and roastwt > 15:
					params = {'RoastDate': date, 'RoastTime': time, 'Job': job, 'Roaster': roaster, 'Item': item, 'Dest_Type': desttype, 'Dest': dest, 'Green_Wt': greenwt, 'Roast_Wt': roastwt, 'Processed': processed}
					system.db.runNamedQuery("Plant", "Insert Record to Roaster Production", params)
					break

Should be something more like this, although personally I think this script is better suited in a Gateway Tag Change Event.

def valueChanged(tag, tagPath, previousValue, currentValue, initialChange, missedEvents):
	if currentValue.value:
		tagPaths = ["[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_DATE",
					"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_TIME",
					"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_BANO",
					"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_1_",
					"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_3_",
					"[~]GA_STATUS_40_",
					"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_4_",
					"[~]Program:MainProgram/RT_ROAST_AUTO_DEST",
					"[~]R1_Bag_Count"]
		qVals = system.tag.readBlocking(tagPaths)
					
		date = qVals[0].value
		time = qVals[1].value
		job = qVals[2].value
		roaster = 1
		item = qVals[3].value
		destcheck = qVals[5].value
		greenwt = qVals[4].value
		processed = 0
		destcheck = qVals[5].value
		roastwt = qVals[6].value
		if destcheck == 1:
			desttype = "Silo"
			dest = qVals[7].value
			if dest != 0 and roastwt > 15:
				params = {'RoastDate': date, 'RoastTime': time, 'Job': job, 'Roaster': roaster, 'Item': item, 'Dest_Type': desttype, 'Dest': dest, 'Green_Wt': greenwt, 'Roast_Wt': roastwt, 'Processed': processed}
				system.db.runNamedQuery("Plant", "Insert Record to Roaster Production", params)
		else:
			desttype = "Bag"
			dest = qVals[8].value
			if dest != 0 and roastwt > 15:
				params = {'RoastDate': date, 'RoastTime': time, 'Job': job, 'Roaster': roaster, 'Item': item, 'Dest_Type': desttype, 'Dest': dest, 'Green_Wt': greenwt, 'Roast_Wt': roastwt, 'Processed': processed}
				system.db.runNamedQuery("Plant", "Insert Record to Roaster Production", params)
1 Like

So, here is your script, simplified a bit:

def valueChanged(tag, tagPath, previousValue, currentValue, initialChange, missedEvents):
	if currentValue.value:
		paths = [
			"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_DATE",
			"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_TIME",
			"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE_BANO",
			"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_1_",
			"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_3_",
			"[~]GA_STATUS_40_",
			"[~]Program:MainProgram/RO/RO_1_/BATCH_DSCALE/BATCH_DSCALE_4_",
			"[~]Program:MainProgram/RT_ROAST_AUTO_DEST",
			"[~]R1_Bag_Count"
		]
		date, time, job, item, greenwt, destcheck, roastwt, silo, bag = [qval.value for qval in system.tag.readBlocking(paths)]
		roaster = 1
		processed = 0
		while True:
			if destcheck == 1:
				desttype = "Silo"
				dest = silo
			else:
				desttype = "Bag"
				dest = bag
			if dest != 0 and roastwt > 15:
				params = {
					'RoastDate': date,
					'RoastTime': time,
					'Job': job,
					'Roaster': roaster,
					'Item': item,
					'Dest_Type': desttype,
					'Dest': dest,
					'Green_Wt': greenwt,
					'Roast_Wt': roastwt,
					'Processed': processed
				}
				system.db.runNamedQuery("Plant", "Insert Record to Roaster Production", params)
				break

Here, no logic has been changed, it's a simple refactor/cleanup.

  • batched tag reads, because it's more efficient and more readable
  • grouped variable assignations
    also removed the roastwt=0 line as it's overwritten by a tag read,
    removed dest and destType as they're assigned later anyway.
  • refactored the inside of the loop to make it avoid repetition and billion characters lines (nobody likes scrolling horizontally).

Now, about what's going on there.
The loop is useless. Nothing will make currentValue.value change, so either it enters, or it doesn't.
If it enters, and one of the if condition is satisfied, it breaks out. If not, it will loop infinitely. Something must change there.

Tell us more about what you thought the loop would do, and we'll try and help you fix it.
Also, are you a C programmer ? This looks like a C programmer trying to code in python.

5 Likes

I was using a while loop because sometimes there is a slight delay from when the gate opens (the bit the change script is on) and when the tag for roastwt & dest is populated with data.

The original script that was (kind of) working would occasionally log a record with 0lbs roastwt and/or 0 as the destination. So, my thinking was that if I keep reading those tag until the data came in, it would ensure I don't log null data.

*I added the dest = 0 / roastwt = '' etc because someone told me that I have to declare those vars outside of the IF statements.
*I am relatively new to programming and most of my experience is in FactoryTalk View VB, have been working with Python in ignition for the last few months or so.

Oh man, do I feel your pain. Welcome to a whole new world, where things aren't stupid. :rofl:

The script should probably be in a Gateway Tag Change event, on the Tag that actually triggers the data collection.

Then you can collect prerequisite data and record it when you need to. There is potential here to miss events, Gateway Tag Change Events will not miss events.

5 Likes

That's only true if you want the variables to be accessible outside of the if's scope, in case the if is not entered, in which case the variable would never be declared.

if something:
    x = 42
print x
# if something was false, then x was not declared, and you can't print x
if something:
    x = 42
else:
    x = 21
print x
# whether something was true of false, x was declared and given a value.

A 'fake' infinite loop, especially with no delay, is not the way. The solution will depend on the details of your setup.
I'm thinking @lrose is right, if the script sometimes triggers at the wrong time, then it's the wrong trigger. Find what event signals everything is in order for you to start building the query and running it, and use that. A event can also be multiple events bound together.

2 Likes

Thank you guys, very helpful!! I am going to start by rewriting this as a Gateway tag change script and go from there.

Unfortunately, the bit I am monitoring to trigger this is the only data point I have to monitor to know it's time to log the data. The delay in the other tags is due to some funky logic in the existing PLC program which I am unable to edit so I have to work around this slight delay.

So, to clarify, all tags besides roastwt and dest have a value before the gate opens.
The desttype may be subject to change up until the moment the gate opens.
Only when the gate opens (the tag I'm monitoring), do I know I have most of my data and after that, I have a limited window of time before the data in the tags I'm reading is cleared out. I have noticed that usually within a few seconds of the gate being open, the other tags are populated with the correct data.

Is there a way to add a delay in the script? like maybe just a few seconds so I don't have to mess around with looping?

There is surely a be a better way, but I'd break this up into different data change scripts. I would think you'd want a script for data collection (for all data accessible when the gate opens). I would log those tags to Ignition memory tags if you think they won't be reliable by the time the 'dest' PLC tag isn't 0. Then I'd create a data change script with a boolean expression tag trigger. That script would be for running the Named Query only. Boolean expression would be defined by Dest != 0 (Ignition tag equivalent) && roastwt > 15 (Ignition tag equivalent) && (A flag that sa the other script has run).

2 Likes

Maybe add a trigger tag ?
Something like an expression tag, with an expression of this sort:
{gateOpens} && {roastwt} > 15 && ({siloDest} != 0 || {bagDest} != 0)
Then monitor that trigger tag.

This is probably not the best expression for this, but it gives you an example of using multiple events as a trigger.

Maybe a loop actually was the simplest way to do this, but...

4 Likes

Remember, this is a tag change script, there is no requirement for it to be a Boolean value. The tag is in ignition, therefore you can monitor it for a change.

2 Likes

So, the issue is really that OPC subscriptions don't guarantee order of arrival of the various values. Your trigger's True can arrive before the others even though, in the PLC, they happen in order.

The solution is to not use tags for the dependent values. Use a tag for the trigger and in the change event, use system.opc.readValues() to make an explicit callout to get fresh copies of the other items.

FWIW, this is precisely what happens if you use the SQL Bridge module's transaction groups with OPC Mode set to "Read". The trigger will be subscribed, the rest will be explicitly read after the trigger fires.

4 Likes

Variables silo and bag needs to use a readblocking or OPC equivalent in the while loop. Those will not get updated otherwise, and you will remain in the while loop forever.

Usually I would avoid while(True), but if required so I would use a timeout. I would also add a else with a time.sleep(0.1) for the "if dest !=0 and roastwt > 15:" to avoid have a cpu at 100%

import time

timeout = time() + 10 # 10 seconds timeout

while time() <= timeout:
    # ... do something ...
    if dest != 0 and roastwt > 15:
        # ... do more things ...
        break
    else:
        time.sleep(0.1)
1 Like

Using indefinitely repeating loops and/or sleep() in UI events is simply crazy in this platform. They don't scale, and you'll regret it when you need to scale.

1 Like

Is the tag value changed script in the UI thread?

1 Like

Actually, they don't scale for tag events either, since there is a limited number of threads to execute all tag events.

1 Like

I know, I thought I wasn't changing the logic of the initial code.
I did not realize he was reading a tag there because he was 'waiting' for its value to change.

1 Like

UPDATE: I made a bool expression tag for each of the roaster machines that are true when (the gate is open && Roastwt > 0 && (dest != 0 || desttype = bag)). I made a Gateway tag change script that monitors the expression tag (pretty much exactly how you guys rewrote my original script above @lrose @pascal.fragnoud). When triggered, it is logging data now but it is logging the same data multiple times.

Is there a way to stop the script after I run the query? or should I edit the DB table I'm writing to and make the date/time/roaster# primary keys?