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

Switch bugfixes #641

Merged
merged 2 commits into from
Dec 22, 2020
Merged

Switch bugfixes #641

merged 2 commits into from
Dec 22, 2020

Conversation

Neverhorst
Copy link
Member

Description

Fixed a bug in the switch interface caused by decorating not only the property but also the setter function as abstractmethod.

Removed explicit states property declaration from dummy module to showcase the working change.

Fixed a bug in the switch logic causing the watchdog to not register switch state changes.

How Has This Been Tested?

Win10 dummy

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • I have documented my changes in the changelog (documentation/changelog.md)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added/updated for the module the config example in the docstring of the class accordingly.
  • I have checked that the change does not contain obvious errors (syntax, indentation, mutable default values).
  • I have tested my changes using 'Load all modules' on the default dummy configuration with my changes included.
  • All changed Jupyter notebooks have been stripped of their output cells.

… property but also the setter function as abstractmethod.

Removed explicit states property declaration from dummy module to showcase the working change.

Fixed a bug in the switch logic causing the watchdog to not register switch state changes.
Copy link
Member

@kay-jahnke kay-jahnke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested on dummy and saw the error. This fixes it.

One question though, what is the purpose of the wrapper @interface_method then?

@Neverhorst
Copy link
Member Author

@kay-jahnke the purpose of @interface_method is still that you can overload the method (or property) using qudi interfaces and Connector objects.
I'm not sure that this will work flawlessly with Python properties since interface_method was not designed to work with them (but apparently it works anyways to some extent).
I would assume it works with read-only properties but will get namespace issues if trying to define a setter method for an interface-overloaded property using the decorator approach, e.g. with @<property_name>.setter. Needs testing to be sure.

Issue with this particular problem was, that the property setter must not be an interface method (only the property itself which is then refering to the setter)

@Neverhorst Neverhorst merged commit 4752917 into master Dec 22, 2020
@Neverhorst Neverhorst deleted the switch_interface_bugfix branch December 22, 2020 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants