Modbus - getting multiple reads when I only want one

Code Samples and Tips
1

This is a tough one, but I know people smarter than me are here to help!

Background and Configuration
I have integrated Opto22’s Modbus Integration Kit ( http://www.opto22.com/site/downloads/dl_drilldown.aspx?aid=4206 ) in the attached PAC Control Professional 9.6d project. For the modbus server, I am using “Modbus Server Pro” v3.3 for Mac, Modbus Server - AppHugs.

To make the project run in your environment, you will need to change the string variables sIPPORT_CYCLOTRON and sIPPORT_TMS initial value to the IP address and port of your modbus server. To get the dasCyclotronFastData chart to run, you will need to set the modbus input register 901 to 32.

Goal
Read from modbus many registers at different time intervals, and write those values to files. Some data needs to be read at a “fast” interval, every 5 seconds. Some data at a “slow” interval, every 60 seconds. I have created charts for fast and slow data reads.

Problem / Bug / Issue
This issue I am having is, in chart dasCyclotronFastData, function block 76 Tuner Position, is writing data multiple times (up to 10 times!), but I am only reading the data once.

All other blocks in that chart are working correctly, reading the data once and writing once. The dasCyclotronFastData chart writes to c:\Datastore\das_fast_data.txt. That is where the multiple values for “tuner position” are showing up. The dasCyclotronSlowData chart writes to c:\Datastore\das_slow_data.txt. There are no duplicates in that file. If any chart has an error in reading data, they all write to c:\Datastore\das_error_data.txt.

The dasCyclotronFastData chart reads once every 5 seconds. The dasCyclotronSlowData chart reads once every 60 seconds.

Request / Help / Guidance
First I am asking is why is function block 76 Tuner Position writing data multiple times, when the other blocks are not?
What I am also requesting is architecture and programming guidance. Is this the best way to structure the code to meet the goal?

This code is hard to maintain. If we add additional elements to capture, we have to add additional blocks. Is there a way to use tables to loop through the data? Could subroutines be created?

Do not worry about hurting my feelings or stepping on my toes! Honest and constructive feedback is what I am asking for to help me learn.

Thank you community.

data.zip (7.0 KB)
pcs.Archive.D11302017.T111106.zip (81.9 KB)

2

First of all, I like the way you put the underscore in your text tool - looks sharp.

Not sure why you are getting the duplicated entry - I don’t see anything obvious and I haven’t ran your project to see what happens, just looked through your chart. I’m sure you will track it down- you are learning fast. Perhaps the following suggestion will help you refactor your code a bit and the bug will go away or be easier to track down…

You could definitely create a subroutine for these calls. There is a computer science term called DRY, which means “Don’t repeat yourself”.

When you find yourself writing the same code over and over, you should stop and ask yourself if you could put this into a subroutine (or create a loop and use pointers/tables - a bit more advanced)

So after reviewing two of your blocks (RF Frequency and Tuner Position) I find these are the only significant differences:

Modbus register
Error message name and text (could you use the data label constant here too?)
Data label constant (e.g. sTUNERPOSITION)

So I think you could have a subroutine something like:

ReadandRecordData(nModbusRegister, sLabel);

You could call this for every point:

ReadandRecordData(nModbusRegister1, sLabel1);
ReadandRecordData(nModbusRegister2, sLabel2);
ReadandRecordData(nModbusRegister3, sLabel3);

and even better you could put the parameters in tables and create a loop to call the subroutine:

for i=0 to nLastIndex step 1
  ReadandRecordData(ntRegisters[i], stLabels[i]);
next

Now when you need to add another item, you add it to the register and label tables and increase nLastIndex.

So, I may be over-simplifying a bit (as you have different modbus function codes to deal with and probably some other things), but that would reduce 22 blocks of code to a few lines + subroutine and an initialization block for your tables. Also, you will only have one subroutine to debug instead of 22+ blocks.

3

Thanks, @philip!

Here’s a bit more on the joy of subroutines: PAC Control 201: Subroutines! (Roll your own command)

Also one on pointers: PAC Control 101: Why/when/how would I use POINTERS?

On the topic of Maintainable code, you might get a kick out of this video that Ben & I did a while back: http://www.opto22.com/players/w_player.aspx?vid=67sc67zfrj&pid=cifwp2ccee&sec=1&vi=2

I hope that helps!