S7NetPlus / s7netplus

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

Sync Open() doesn't complete #369

Closed Wobeam closed 3 years ago

Wobeam commented 3 years ago

Hello everyone, In older projects I used v.0.1.9 for them it was taken already assembled dll which had location s7netplus-0.1.9/nuget-pack/lib/net45/s7net.dll. It worked decently. Today, just out of curiosity I've tried to add to solution S7.Net class library. Then made needed references to S7.Net class library. To my surprise there's a huge difference at behaviour. Seems this works but it is so faulty and unstable comparing to dll which was assembled by developers.

No matter, I assembled dll from provided S7.Net class library and made a reference to dll - got the same situation.

I believe I did something wrong. But what?

Any comments would be appreciated.

mycroes commented 3 years ago

I have no idea what differences you're talking about, please explain...

Wobeam commented 3 years ago

Ok,

I have no clue what could be a difference between these DLLs.

What could be a reason of such misbehaving? These all were taken from the same release s7netplus-0.1.9 .
Is there any specifities which I should consider while assembling?

mycroes commented 3 years ago

Not that I know of. There might be differences due to debug or release configuration and probably you're using a newer compiler than what 0.1.9 was built with, although I doubt that would matter much.

However, 0.1.9 is three years old. It has a few defects that have been fixed and overall reliability should be better, so please just use a recent release instead.

Wobeam commented 3 years ago

@mycroes, hello followed your advice and ended up with dll v7.0. the next version 8.0 has thrown a fail as per picture attached. And there isn't data exchange at all. What could be a reason or difference between 0.1.8 - 7.0 and v.8.0 that cause a fail?

thanks. all threads

mycroes commented 3 years ago

The latest is 0.8.1, and what you're showing is not a failure (did you even read the message?) I can imagine however that you're using IsAvailable to check for the connection state, which you shouldn't be doing anyway. No property check can ensure that the next call will succeed, so just make the call and handle exceptions appropriately. IsAvailable was breaking the connection for a long time, I'm unsure about it's current state.

Wobeam commented 3 years ago

@mycroes, this message I read and it said me nothing useful

The function evaluation requires all threads to run.

I'm not using IsAvailable at all.

With the simplest connection code below I went through all available Dlls beginning with 0.1.8 and ending with 7.0 . They are all workable. The data exchange takes place with all of them.

            try
            {
                plc = new Plc(CpuType.S71500, "100.1.1.70", 0, 0);

                plc.Open();

            }

            catch (Exception ex)
            {

                MessageBox.Show(ex.Message);

            }

However, beginning with v.8.0 the piсture is changing to the opposite, There are not data exchange and any exceptions appeared on the screen. The picture while debugging has been shown in my previous post.

Please help.

mycroes commented 3 years ago

I guess you're making these calls from the ui thread, there might be an issue there with regards to how the sync api now just invokes the async api. Could you change the method to be async and use the OpenAsync method? If that works, I have an idea about what to look for.

Wobeam commented 3 years ago

@mycroes, if I got you correctly,

I have done as follows:

        private async void Connect()
        {

            try
            {

                plc = new Plc(CpuType.S71500, "100.1.1.70", 0, 0);

                await plc.OpenAsync();

            }

            catch (Exception ex)
            {

                MessageBox.Show(ex.Message);

            }

        }

However, the issue still persists. UI appears, there's no data exchange there.

mycroes commented 3 years ago

That's right. Could you try to connect using a command line application to see if the issue relates to the fact that this is on the ui thread at all?

scamille commented 3 years ago

Hmm so you suspect that the different synchronization context in a gui application might break the sync calls? We probably never tested that, since unit tests behave like command line applications.

Unfortunately I don't have that much experience/knowledge about this. Shouldn't ensuring that there is ConfigureAwait(false) used everywhere internally fix the problem?

Wobeam commented 3 years ago

@mycroes, again, if I got you correctly,

this picture below, the test has been made with v7.0.

ConsoleApp_basedOn_v 7 0

this picture below, the test has been made with v 8.0

ConsoleApp_basedOn_v 8 0

What address is out of range? Is there any key difference between 7.0 and 8.0?

Update: seems found something. A culprit is reading of Reals.

So... please enlighten me how to handle properly with reals. Since at v8.0 you replaced Double type with LReal (according to description).

scamille commented 3 years ago

S7 Real maps to .NET float / System.Single, while LReal maps to .NET double / System.Double

Wobeam commented 3 years ago

Ok, thanks. what about integers? if Plc side has Int or DInt

What about .Net side?

mycroes commented 3 years ago

Types of matching size, so: Int => System.Int16 or its alias short, DInt => System.Int32 or its alias int.

Wobeam commented 3 years ago

Hello, at early version there have been done an opportunity to read Error Codes as follows: if (errCode != ErrorCode.NoError) throw new Exception(plc.LastErrorString); It was quite helpful. What about the lattest ones? They don't have that. So how to handle with such kind of errors?

thanks.

mycroes commented 3 years ago

That's been changed to throw relevant exceptions internally. Returning error codes and LastError properties are more C-like, they're not considered good practice in C#. Changes have been made to make the library more conformant with what is common practice in the C# world.

Mallowan commented 3 years ago

@Wobeam, I would note that reasonable questions you are asking here. I also had same doubts. Reffering to the fist your post, may I ask how you appreciate this library. Is there something running at industrial area based on this lib and made by you?

Wobeam commented 3 years ago

Hi, you know I used an early version a couple of years ago. This works decently up to now despite of I have been the world's worst C# programmer at that time. Today I am also bad but not so much as two years ago. ) Now there's a task to make something else. Much water has gone under the bridge an we have v8.1 already, here available. I can not tell anything about reliability of the lattest one. The only thing I can tell you that simple replacement of 0.1.9 with 8.1 caused some fails. Currently I'm trying to solve them. Thanks to @mycroes and @scamille the things are moving.

Hope owners or someone experienced find 5 minutes to help me out. ) At the moment I am highly interested in how to address to plc instance from a number of viewmodels and where place plc.Open() method within such a program structure. Of course, I learned it long ago how to create instances of classes and then address to their fields. But this is communication library, and I would like to know how to do that properly in order to get flawless code. I have some my own good practices which I used. However could someone put here a piece of code explaining my question.

thanks.

mycroes commented 3 years ago

@Wobeam I'm actually working on a UI for interacting with PLC's, the part that manages the PLC connection is at src/UI/Models/Plcs/Plc.cs.

It's the first time I'm managing concurrency based on observables, but it works reasonably well. There's probably a 100 approaches you can take to managing concurrency and keeping track of PLC instances, but honestly I think you should be looking for such general advice elsewhere. It's also impossible for me to decide what the best fit for you would be, because I don't write your application...

scamille commented 3 years ago

I had a quick look at the ConfigureAwait settings, and I do think we are all covered.

@Wobeam without an actual error message / stack trace it is hard to understand what your actual problem is. Your picture is just showing some local variables while you are debugging, but nothing about the actual problem you encountered.

Wobeam commented 3 years ago

@scamille, seems I solved issues with types convertation. I left App running and organized constant data exchange for a night. so far there have not been any breaks or UI freezes. So I'm going to encrease traffic load and see. fingers crossed.)

mycroes commented 3 years ago

Closing this issue because it seems fixed now, feel free to continue discussion or open new issues if other problems arise.