Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
491 stars 193 forks source link

Read and Write Strings between TwinCAT3 and Ubuntu on Virtual Machine #194

Closed ismailalhajosman closed 10 months ago

ismailalhajosman commented 1 year ago

Hello,

I have been testing the communication between TwinCAT 3 and Linux OS using ADS libraray. I am able to read and write characters, integers and arrays, however I could not read or write string variables using the example.cpp provided with the libraray

I have searched this isuue a lot and but could not find a solution.

BTW, I was able to read and write strings using python libraray pyads, but I could not do that using c++, that is why I think I am missing something in the C++ code.

I will appreciate your help!

Thanks,

Here is the code I modified depending on example.cpp

1- Read variable function: string read

2- Write variable function: string write

3- Variables on TwinCAT twincat string

pbruenn commented 1 year ago

Hmm, yes. STRING is not implemented for AdsVariable. The Problem with string types in PLC is there are so many of them. You might be able to use SymbolAccess:

auto device = bhf::ads::SymbolAccess{ gw, netid, port };
device.Read("MAIN.sWelcome", std::cout);
device.Write("MAIN.sWelcome", "huhu");
device.Read("MAIN.sWelcome", std::cout);
simonschmeisser commented 1 year ago

I was asked by a customer about this and since it compiled I said "yes it works" ... might have been a bit fast :wink:

Could you please add a bit more details what would need to be added? Maybe I can work on it in a PR then?

pbruenn commented 1 year ago

@simonschmeisser Take a look into https://github.com/Beckhoff/ADS/blob/master/AdsTool/main.cpp#L619

If your PLC datatype is T_MaxString you can use the code path for "STRING":

auto device = AdsDevice { gw, netid, port ? port : uint16_t(AMSPORT_R0_PLC_TC3) };
const auto handle = device.GetHandle("MAIN.sWelcome");

std::vector<uint8_t> readBuffer(size);
uint32_t bytesRead = 0;
const auto status = device.ReadReqEx2(ADSIGRP_SYM_VALBYHND,
                                              *handle,
                                              readBuffer.size(),
                                              readBuffer.data(),
                                              &bytesRead);

// readBuffer.data() contains the value of "MAIN.sWelcome"
std::cout.write((const char*)readBuffer.data(), bytesRead);

// MAIN.sWelcome = "huhu"
const auto value = std::string("huhu");
device.WriteReqEx(ADSIGRP_SYM_VALBYHND,
                                     *handle,
                                     value.size() + 1,
                                     value.data());
simonschmeisser commented 10 months ago

SymbolAccess seems to override only the length of the new string, but if the previous string was longer the rest remains:

device.Write("MAIN.sWelcome", "hello world");
device.Write("MAIN.sWelcome", "b");
device.Read("MAIN.sWelcome", std::cout);

prints bello world

pbruenn commented 10 months ago

@simonschmeisser can you give feedback on this attempt to fix: https://github.com/Beckhoff/ADS/tree/patrickbr/fix-strings.

simonschmeisser commented 10 months ago

@pbruenn that seems to work well, thanks a lot! :+1: If you add 0x1E here: https://github.com/Beckhoff/ADS/blob/81a402053a19365ba546ec0dc29331b245f9eb71/AdsLib/SymbolAccess.cpp#L283 we would get rid of the warning below

simonschmeisser commented 10 months ago

One more thing: If the string is longer than the char array on Beckhoff side (80 in my case) the PLC will be unhappy again as the string needs to be zero-terminated. So I added

buffer[entry.header.size-1] = 0;

after the resize and TwinCat will display the truncated string nicely.

Of course one could also throw an exception if the string is too long, would be more proper I guess

pbruenn commented 10 months ago

@pbruenn that seems to work well, thanks a lot! 👍 If you add 0x1E here:

https://github.com/Beckhoff/ADS/blob/81a402053a19365ba546ec0dc29331b245f9eb71/AdsLib/SymbolAccess.cpp#L283 we would get rid of the warning below

Well, on my systems 0x1E is Tc2_System.T_MaxString so yeah we could add 0x1E to the switchcase, but is it always correct? My PLC knowledge and experience it to low to answer this confidently with "YES".

pbruenn commented 10 months ago

One more thing: If the string is longer than the char array on Beckhoff side (80 in my case) the PLC will be unhappy again as the string needs to be zero-terminated. So I added

buffer[entry.header.size-1] = 0;

after the resize and TwinCat will display the truncated string nicely.

Of course one could also throw an exception if the string is too long, would be more proper I guess

Well, what do we do when we fit exactly? eg the string has the exact same length as the array but we cannot fit the zero terminator? In my opinion we can only check for buffer overflow but not for missing null terminator because we have no always valid list of types which we should consider to be a STRING.

simonschmeisser commented 10 months ago

You are the one working at Beckhoff, right? :laughing:

It's certainly not the only form of string but be we can still add more later as we learn about them?

pbruenn commented 10 months ago

You are the one working at Beckhoff, right? 😆

It's certainly not the only form of string but be we can still add more later as we learn about them?

no we can't because the dataType id is not unique across all PLC instances for custom data types. So 0x1E could mean " Tc2_System.T_MaxString" on one PLC but "MyCustomLibrary.Structur" on another. I know of no method to detect if a datatype is some kind of string. It might be possible to do recursive lookups for constructed types like that but it will become much more complex than simply adding another dataType id to a switch case. EDIT: fixed markdown format ...

pbruenn commented 10 months ago

This bug should be fixed by https://github.com/Beckhoff/ADS/commit/ecac634ec402a162627af4b9a31ab09ad301be4c.