Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
513 stars 194 forks source link

dev-ooi: AdsClient.h does not compile with Visual Studio 2015 #38

Closed pbruenn closed 7 years ago

pbruenn commented 7 years ago

moved from #35: xift wrote: Sidenote: AdsClient.h does not compile with Visual Studio 2015. It doesn't like the HandleGuard.

return AdsHandleGuard {new uint32_t {handle}, std::bind(&ReleaseHandle, address, port, std::placeholders::_1)};

No instance of constructor "std::unique_ptr<_Ty, _Dx>::unique_ptr [with _Ty=uint32_t, _Dx=std::_Binder &>]" matches the argument list. Argument types are: (uint32_t , std::_Binder<std::_Unforced, void ()(AmsAddr address, long port, uint32_t *handle), const AmsAddr &, long &, const std::_Ph<1> &>) You seem to be quite familiar with modern C++. Do you often use ? I know what bind does but I have never had the opportunity to use it. I just don't think about that. How did you start using it?

pbruenn commented 7 years ago

I will take a look on this one later. Maybe VC++ is missing some c++11 feature or my knowledge is smaller than you think and gcc is just doing magic with my code ;-). Some years ago I attended "ESE Kongress" in Sindelfingen. The lecture of Prof. Peter Sommerlad opened my mind for C++11 and simple code. So I started to take a deeper look into STL and always try solve my problems using standard C++ library mechanisms. Funny fact: he told me at the coffee table destructors are evil. At that time I didn't really understand what he meant. But now I "think" I figured out it is because of the "rule of five", so I try to implement "rule of zero". At least AdsHandleGuard is my try to implement it. Comments and laughter of experts would be appreciated ;-).

Regards, Patrick

mwiarda commented 7 years ago

The ironic thing is that what prevents me from using the STL in my daily work is the limited subset of the STL that is supported by TwinCAT 3.1 C++ modules :-) I try to keep myself up to date by listening to conference talks and reading a lot. Whenever I work on a "non-kernel-mode" sideproject I test out everything I learned.

That rule of zero thing is a really clever idea. I already admired that solution in your code 👍 Its beautiful - but it is not obvious. I like code to be straightforward and easy to read. This solution is neat but can throw off people quickly. You always stop to wonder if it really covers any remote case that can happen. Anyway, it probably outperforms any constructor/destructor/... implementation I could write.

I guess you are right with the rule of five idea. And then there is the "throwing in destructors"-problem and the issue with destructors of base classes.

Regards, Michael

pbruenn commented 7 years ago

:laughing: I will try to convince my colleagues of c++14 support in TwinCAT at the lunch table ;-).

I prefer easy to read code, too. But that lecture encouraged me "learn to master the tools you use". In every profession you have to learn to handle your tools. Only in programming some guidelines force you to write stuff like this:

if (0 == error) {
    do_error_handling(error);
} else {
    // this is empty to be compliant with coding guideline...
}

Just to make sure anyone could read it. No one would tell a carpenter to always use a handsaw just because a circular saw will cut his finger faster.

mwiarda commented 7 years ago

I rather have no C++14 support than some custom implementation by Beckhoff (No offense ;-)). There are already some pitfalls like the implementation of references. References in TwinCAT modules have their own address. I actually checked - The standard doesn't say anything about that. In every other system &refX == &X. Not in TwinCAT. Or the sprintf function that didn't support floats until recently.

It's not only in programming with the guidelines. Just look at the colour coding of cables in electronics. It is safer if everyone understands it. I agree with your tools argument but also like the "Don't be too clever" rule for beautiful code.

Regards Michael

pbruenn commented 7 years ago

I am absolutely with you. "Clever coding" is as stupid as "Clever cable handling". However if you want to be allowed to handle cables, you _haveto learn the cable codings.

In commit a4f206449f41bca2dbadff196b99819018979d06 you can see even clever "rule of zero" has its pitfalls ;-).

I still think there is some error in VC++2015 std::bind/placeholders/std::unique_ptr implementation. This article sounds promising but even with VS2015 Update 3 I got the same error so I rewrote AdsHandle in commit b1d306e9b0eddccd6eb0b1ea541120655a5500c8