Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
517 stars 197 forks source link

examples refactored #31

Closed rockonedege closed 7 years ago

rockonedege commented 8 years ago

I did some refactoring my branch. I'm not sure if you are interested or OK with the style. My changes are mainly for easy adaption to new remote targets and grouping of similar functions.

Eventually, I'd like to use this lib to build a remote HMI, be it CLI or GUI, starting from this example. By the way, do you have an example for ADS-sum command? is it supported in this implementation?

pbruenn commented 8 years ago

Thanks for sharing, I need such input to get a better impression on how the AdsLib is used by the applications and adapt the interface for it. I like the auto_handle and will keep that in mind, when we improve the interface (#26) and examples. And I see the benefits of your namespaces and the out parameter reordering, but I will not adapt it for the example, now. In my opinion the benefit is to small to justify the big diff. A general note to the style: To make it easier to keep just one code style we have an uncrustify config within the repository. You can run it manually with make uncrustify. Or even better use a pre-commit hook. You can install one with make prepare-hooks. Just keep that in mind, if you plan to send a patch or pull request.

"example/" was intended to show how to integrate AdsLib with your own application. For examples on how to use the ADS functions we always referenced Beckhoff Infosys. However, as you are not the first one(#3, #24) asking for ADS-sum command, I believe we should start to bring more examples here ;-). But don't expect that to happen within the next three weeks, as I will be unavailable for that time.

rockonedege commented 8 years ago

Thanks for responding. I'll be keeping updating changes in my fork, first the interfaces, and then the library if necessary. If there's anything you see value and would like to include in the upstream. Feel free to let me know. I don't mind patching based on your code base, though I 'll take liberty in my fork.

I just noticed uncrustify while creating a CMake file studying your make file. It's an interesting tool I'll add it to the CMake script if possible.

For the sum command, yes please add it. No hurry though, keep it in your pace.

pbruenn commented 7 years ago

I absolutely see a point in having examples close to the code. So adding them here would definitely make live easier for all users of the open source AdsLib. However, for the majority of ADS developers are using the Windows libraries provided with TwinCAT and I am a strong opponent of redundancy. So after a lot of discussion, we decided to keep referencing to Beckhoff Infosys. If you have feedback to these examples, let us know and we try to improve them there for the benefit of all ADS users.