Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPC DA: Upgrade to OPCClientToolKit with 64bit support #37

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

kumajaya
Copy link

@kumajaya kumajaya commented Mar 7, 2024

Depend on 32/64bit OPCClientToolKit:
https://github.com/kumajaya/OPC-Client-X64.git

This upgrade includes:

  • Source code adjustments due to library changes
  • Completely disconnect a client from the server when disconnecting
  • Remove FB group and items on terminated
  • Update installation instructions

Tested and working on Windows 11 and Windows 7 64bit with Matrikon OPC Simulator, Festo Didactic EzOPC, Moxa OPC Server (with a real hardware I/O module).

@kumajaya
Copy link
Author

kumajaya commented Mar 9, 2024

Found a bug: write requests don't work after reinitializing FB on shared a connection. On terminated, the FB group and associated items must be deleted and will be re-registered at the next initialization. This is not a new bug but a bug from the previous source.

@azoitl
Copy link
Contributor

azoitl commented Mar 10, 2024

Found a bug: write requests don't work after reinitializing FB on shared a connection. On terminated, the FB group and associated items must be deleted and will be re-registered at the next initialization. This is not a new bug but a bug from the previous source.

As so often you where faster then me. I was about to write if this bug fix should be an independant PR?

@azoitl
Copy link
Contributor

azoitl commented Mar 10, 2024

In order to get the Eclipse Foundation's IP process started I filed an IP Lab issue to get clearance for this upgrade.

During writing this IP Lab issue I noticed that you forked the lib. Does it make sense that you contribute your changes back to the origin and 4diac FORTE uses the original version?

@kumajaya
Copy link
Author

Found a bug: write requests don't work after reinitializing FB on shared a connection. On terminated, the FB group and associated items must be deleted and will be re-registered at the next initialization. This is not a new bug but a bug from the previous source.

As so often you where faster then me. I was about to write if this bug fix should be an independant PR?

Yes, it would be better as a separate PR but I haven't done enough testing on the previous implementations, the problems that occur now may come from previous work but that could just be my own assumptions.

@kumajaya
Copy link
Author

In order to get the Eclipse Foundation's IP process started I filed an IP Lab issue to get clearance for this upgrade.

During writing this IP Lab issue I noticed that you forked the lib. Does it make sense that you contribute your changes back to the origin and 4diac FORTE uses the original version?

I will. Currently having a strange inconsistent issue that I believe is related to string conversion. Give me some time to fix it.

@azoitl
Copy link
Contributor

azoitl commented Mar 10, 2024

In order to get the Eclipse Foundation's IP process started I filed an IP Lab issue to get clearance for this upgrade.
During writing this IP Lab issue I noticed that you forked the lib. Does it make sense that you contribute your changes back to the origin and 4diac FORTE uses the original version?

I will. Currently having a strange inconsistent issue that I believe is related to string conversion. Give me some time to fix it.

Take the time you need. I think currently you are our main users of OPC DA. I'm very happy that you spend the time on improving both sides.

@kumajaya
Copy link
Author

In order to get the Eclipse Foundation's IP process started I filed an IP Lab issue to get clearance for this upgrade.
During writing this IP Lab issue I noticed that you forked the lib. Does it make sense that you contribute your changes back to the origin and 4diac FORTE uses the original version?

I will. Currently having a strange inconsistent issue that I believe is related to string conversion. Give me some time to fix it.

Take the time you need. I think currently you are our main users of OPC DA. I'm very happy that you spend the time on improving both sides.

Thank you for your kind words. About string conversion, often I get unexpected results in a tight loop conversion and I've found a way to fix it. Module refactoring needed to reducing string conversions but probably will be done after this PR. As a side note, I also noticed an unreadable log issue in the MQTT module.

@kumajaya kumajaya force-pushed the upgrade/opcda branch 2 times, most recently from 4a7eb23 to 1d88ee7 Compare March 15, 2024 15:24
@kumajaya
Copy link
Author

kumajaya commented Mar 16, 2024

In order to get the Eclipse Foundation's IP process started I filed an IP Lab issue to get clearance for this upgrade.

During writing this IP Lab issue I noticed that you forked the lib. Does it make sense that you contribute your changes back to the origin and 4diac FORTE uses the original version?

I just create a PR to the origin: edimetia3d/OPC-Client-X64#18 , please check it if you still have any concern. Thank you.

@kumajaya
Copy link
Author

@azoitl Sir, I just created a pre-release of OPCClientToolKit library so users could enable OPC DA support without compiling the library needed from source: https://github.com/kumajaya/OPC-Client-X64/releases From my side, this PR has been completed.

@azoitl
Copy link
Contributor

azoitl commented Mar 16, 2024

@kumajaya thx for the update. I still need to correctly report the new version and update in the IP tracking tool. For me there is one thing not fully clear. Is there now some kind of official OPC lib project. Or are these all forks from the original unmaintained lib. The reason I'm asking is that you also forked an improved lib. Do you consider upstreaming your changes from where you forked? Should your origin be the then the things users should use?

@kumajaya
Copy link
Author

kumajaya commented Mar 17, 2024

@kumajaya thx for the update. I still need to correctly report the new version and update in the IP tracking tool. For me there is one thing not fully clear. Is there now some kind of official OPC lib project. Or are these all forks from the original unmaintained lib. The reason I'm asking is that you also forked an improved lib. Do you consider upstreaming your changes from where you forked? Should your origin be the then the things users should use?

To be honest I'm not sure if the maintainer is aware of my PR as there has been no activity for about two years. In fact, it's actually a common thing in the open source world that other people then continue maintain a version according to their needs. In the meantime I will maintain my forked version (depending on my capabilities) on my own repository as a develop branch until my PR is accepted. I've also created my own 32 and 64bit binary releases to make it more easier to enable OPC DA support to FORTE.

src/com/opc/opcconnectionimpl.cpp Outdated Show resolved Hide resolved
src/com/opc/opcconnectionimpl.cpp Outdated Show resolved Hide resolved
src/com/opc/opcconnectionimpl.cpp Outdated Show resolved Hide resolved
src/com/opc/opcconnectionimpl.cpp Outdated Show resolved Hide resolved
@azoitl
Copy link
Contributor

azoitl commented Mar 25, 2024

Created dedicated IP Lab issue to correctly handle dependencies: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/14078

@kumajaya
Copy link
Author

@azoitl Sir, I just managed to make OPC DA 64bit build working on Windows 7. FORTE as an OPC DA to OPC UA gateway for my old DCS can truly will be realized. So excited! Thank you.

@azoitl
Copy link
Contributor

azoitl commented Mar 26, 2024

@azoitl Sir, I just managed to make OPC DA 64bit build working on Windows 7. FORTE as an OPC DA to OPC UA gateway for my old DCS can truly will be realized. So excited! Thank you.

Wow this is great! If you want I would be happy about a guest news post on your work for the Eclipse 4diac webpage.

Depend on 32/64bit OPCClientToolKit:
https://github.com/kumajaya/OPC-Client-X64.git

This upgrade includes:
* Source code adjustments due to library changes
* Completely disconnect a client from the server when disconnecting
* Remove FB group and items on terminated
* Update installation instructions
@kumajaya
Copy link
Author

In order to get the Eclipse Foundation's IP process started I filed an IP Lab issue to get clearance for this upgrade.
During writing this IP Lab issue I noticed that you forked the lib. Does it make sense that you contribute your changes back to the origin and 4diac FORTE uses the original version?

I will. Currently having a strange inconsistent issue that I believe is related to string conversion. Give me some time to fix it.

Take the time you need. I think currently you are our main users of OPC DA. I'm very happy that you spend the time on improving both sides.

I can confirm with the latest changes this PR actually make FORTE's OPC DA module fully working on Windows 11 and Windows 7 64bit (Windows 7 need a fix from a different PR). Read and write multiple items from multiple FB without any problem.

@azoitl
Copy link
Contributor

azoitl commented Mar 26, 2024

By now I also have approval for your PR. Please do not add new changes to it. I will later review it and merge it. Any additional fixes please in new PRs.

@azoitl azoitl merged commit 36a676c into eclipse-4diac:develop Mar 26, 2024
3 checks passed
@kumajaya kumajaya deleted the upgrade/opcda branch March 27, 2024 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants