S7NetPlus / s7netplus

S7.NET+ -- A .NET library to connect to Siemens Step7 devices
MIT License
1.3k stars 580 forks source link

Writing to PLC fails after a while. Application must be restarted. #442

Open hlynsi opened 2 years ago

hlynsi commented 2 years ago

I am using this library in the Siemens RFID reader example application running on an S7-1500 OpenController. When the RFID card is detected in range or leaves range I write data to PLC. This works fine for a while but eventually data is not written to the PLC anymore. When this happens the application has to be restarted and then it writes again to the PLC (for a while).

Does anyone here have a suggestion for the cause?

In the constructor I have

// Read IP address from config file
plcIp = GetIpConfig(test: bool.Parse(ConfigurationManager.AppSettings["testmode"]));

// Used to connect with a PLC to write directly to a Data Block
plc = new Plc(CpuType.S71500, plcIp, short.Parse(ConfigurationManager.AppSettings["rack"]), short.Parse(ConfigurationManager.AppSettings["slot"]));

// Parameterize connection with the PLC Data Block
rfidValue = SetupDataItem(DataType.DataBlock, VarType.S7String, Int32.Parse(ConfigurationManager.AppSettings["rfid_DB"]), 0, 20, Int32.Parse(ConfigurationManager.AppSettings["rfid_Value_offset"]));
rfidTime = SetupDataItem(DataType.DataBlock, VarType.DateTime, Int32.Parse(ConfigurationManager.AppSettings["rfid_DB"]), 0, 1, Int32.Parse(ConfigurationManager.AppSettings["rfid_Time_offset"]));
rfidDet = SetupDataItem(DataType.DataBlock, VarType.Bit, Int32.Parse(ConfigurationManager.AppSettings["rfid_DB"]), 0, 1, Int32.Parse(ConfigurationManager.AppSettings["rfid_CardDetected_offset"]));

// Show on the application the configured IP address.
lblRfidIp.Text = plcIp;

I have defined these methods:

// The IP address for sending to PLC is stored in configuration file.
private static String GetIpConfig(bool test)
{
    if(test)
        return ConfigurationManager.AppSettings["test_ip"];
    else
        return ConfigurationManager.AppSettings["ip"];
}

private static DataItem SetupDataItem(DataType data_type, VarType var_type, int db, byte bit_adr, int cnt, int offset)
{
    return new DataItem()
        {
            DataType = data_type,
            VarType = var_type,
            DB = db,
            BitAdr = bit_adr,
            Count = cnt,
            StartByteAdr = offset,
            Value = new object()
        };
}

I have this where the card is detected

string cardID = serialNumberText.ToString().Split(' ')[2];

logText(String.Format("Card {0} detected in range. ", cardID));
logText(string.Format("Previous detected state is {0}", prevDectedStatus));

logText("Attempting to open a connectiong to the PLC");

plc.Open();

if (plc.IsConnected)
{
    logText("Connection established");

    try
    {
        // The RFID
        rfidValue.Value = cardID;
        rfidValue.Count = cardID.Length;

        // The DateTime datatype must be of the correct S7 format.
        String dt = DateTime.Now.ToString("yyyy-MM-dd-HH:mm:ss");
        rfidTime.Value = DateTime.ParseExact(dt, "yyyy-MM-dd-HH:mm:ss", null);

        // Card in range
        rfidDet.Value = isCardDetected;

        dataItemsWrite.Add(rfidValue);
        dataItemsWrite.Add(rfidTime);
        dataItemsWrite.Add(rfidDet);

        logText(String.Format("Attempting to write rfid={0}, time={1}, detected={2}", rfidValue.Value, rfidTime.Value, rfidDet.Value));
        plc.Write(dataItemsWrite.ToArray());

        logText("Done writing to PLC");
    }
    catch (Exception ex){
        logText(String.Format("Caught an exception: {0}", ex));
    }
}
else
{
    logText("Failed to establish a connection to PLC");
}

logText("Store state (prev=cur), used when card leaves field");
prevDectedStatus = isCardDetected;
logText("Done storing state");

logText("Attempt to close connection to PLC");
plc.Close();
logText("Done, connection is " + (plc.IsConnected ? "open":"closed"));

Finally this is where the card is detected out of range

logText("Card left range");

if (prevDectedStatus != isCardDetected)
{
    logText("Condition met, attempting to open a connectiong to the PLC");
    plc.Open();

    if (plc.IsConnected)
    {
        logText("Connection established");

        try
        {
            rfidDet.Value = isCardDetected;

            logText(String.Format("Attempting to write detected={0}", rfidDet.Value));
            plc.Write(rfidDet);
            logText("Done writing to PLC");
        }
        catch (Exception ex){
            logText(String.Format("Caught an exception: {0}", ex));
        }
    }
    else
    {
        logText("Failed to establish a connection to PLC");
    }

    logText("Store state (prev=cur), used when card leaves field");
    prevDectedStatus = isCardDetected;
    logText("Done storing state");

    logText("Attempt to close connection to PLC");
    plc.Close();
    logText("Done, connection is " + (plc.IsConnected ? "open" : "closed"));
}
hlynsi commented 2 years ago

Instead of opening and closing connection via the same instance of Plc(), would it be any better idea to create a new instance everytime card is detected in/out of range?

Jason-Jelks commented 2 years ago

Is your program continually scanning the PLC for changes? Unless you have connection starvation issues, you could implement IDisposable and open your connection after the constructor is complete then close the connection as part of the Dispose Process. You then don't need to worry about continually opening and closing your connection. But that is only if the option is available for you to maintain the connection.

hlynsi commented 2 years ago

The program does not scan the PLC for changes and the only interaction with the PLC is what I described above. There is no way to predict when the user scans the card.

But the program continuously waits for a card to be read so having the connection open could be a possibility.

I have made a plan for improvements and will share with you how it goes.

Jason-Jelks commented 2 years ago

My apologies, you are writing to the PLC... So then yes you can hold a connection instance open. It should not impact the PLC scan times as you are only writing a small block of information, even if it occurred 20 times per second.

Just because you open the connection, it does not provide you any indication that the connection remains open from one write command to the other, therefore you would not know if the connection closed until the write was attempted. You would need to manage your processes as to how you retry whether that is automated or someone must attempt to rescan.

scamille commented 2 years ago

Your core loop should look something like this:

If (!plc.IsConnected)
  plc.open();
plc.write(...);

That still means that every write call can fail because the connection was closed - because the isConnected function does not and can not actively do that. But once you recover from the exception that occurs in write when the connection is closed you can retry and ensure that the connection is re-established if necessary.

I wouldn't recommend closing the plc each time unless you need to.