-
Notifications
You must be signed in to change notification settings - Fork 96
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
Added fallback to type detection to allow automatic ENUM usage #397
base: master
Are you sure you want to change the base?
Conversation
b249f3d
to
18c777d
Compare
Pull Request Test Coverage Report for Build 10661739215Details
💛 - Coveralls |
70ffe96
to
2eb27f9
Compare
thanks @RobertoRoos , just had a look at this, I'm generally happy with this implementation. Could you just solve the conflict, please. I've also left a comment too in the review. It may be worth a few more tests, either unit or manual, to make sure we haven't broken any other edge cases such as when the user does pass in the type, or the wrong type etc. Also, maybe when structures are passed in, I wonder how it will handle ENUMS in structures....(not sure if that is possible in the struct def actually). Can't see any reason why it would have broken anything else but being paranoid. |
2eb27f9
to
cb41087
Compare
…d exception raising for unknown types
cb41087
to
cf3decc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisbeardy you mentioned more tests might be a good idea. What cases would you like to be covered? I'm not sure what is needed exactly.
+ Added exception raising for unknown types
Fixes #263 . The issue is almost three years old, but still relevant.
This allows using enums both with
plc.read_by_name()
and throughAdsSymbol.read()
.Tested the code with:
Output: