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

ZI UHF-LI: fix scope_mode and scope_average_weight parameters #1597

Merged
merged 2 commits into from
Jun 19, 2019
Merged

ZI UHF-LI: fix scope_mode and scope_average_weight parameters #1597

merged 2 commits into from
Jun 19, 2019

Conversation

qSaevar
Copy link
Contributor

@qSaevar qSaevar commented Jun 13, 2019

Changes proposed in this pull request:

  • Ignore sweeper_sweeptime in snapshot(update=True) if _sweeper_signals is an empty list
  • Getter for scope_mode and scope_average_weight should work

When taking a snapshot of the ZIUHFLI driver with update=True the following warnings are generated:

[ziuhfli(ZIUHFLI)] Snapshot: Could not update parameter: sweeper_sweeptime
[ziuhfli(ZIUHFLI)] Snapshot: Could not update parameter: scope_mode
[ziuhfli(ZIUHFLI)] Snapshot: Could not update parameter: scope_average_weight

This should no longer be the case.

@WilliamHPNielsen

@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #1597 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1597   +/-   ##
=======================================
  Coverage   72.17%   72.17%           
=======================================
  Files         116      116           
  Lines       12416    12416           
=======================================
  Hits         8961     8961           
  Misses       3455     3455

@@ -2265,7 +2278,7 @@ def getduration():
returndict = self.scope.get(querystr)
# The dict may have different 'depths' depending on the parameter.
# The depth is encoded in the setting string (number of '/')
keys = setting.split('/')[1:]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change the change responsible for making the getters for scope_mode and scope_average_weight work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as the setting for scope_mode is just mode and for scope_average_weight is averager/weight then omitting the first part before a / of the setting leaves an empty list for the mode and a key error for the weight

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. And the _scope_getter is only used for the scope_mode, scope_average_weight, and scope_duration parameters, the latter of which is handled explicitly by the _scope_getter function.

The changes look correct. I just wonder why the [1:] was put in to begin with. Perhaps the setting nesting level changed in some firmware upgrade? If you can somehow check that, that would be fantastic. But since these changes make your version work, let's get them in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went over the commit history of the original PR, and it seems that in commit 1066df6b01d6016da8ff7f34ccdfe201aea0fcfd when the special case for duration was added this bug was introduced. Before that commit, rawvalue = returndict was outside and after the while loop while keys != [] and then it should have worked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, good job! Thank you.

@WilliamHPNielsen WilliamHPNielsen changed the title fixing scope_mode and scope_average_weight parameters ZI UHF-LI: fix scope_mode and scope_average_weight parameters Jun 17, 2019
@WilliamHPNielsen WilliamHPNielsen merged commit 22b6d88 into microsoft:master Jun 19, 2019
giulioungaretti pushed a commit that referenced this pull request Jun 19, 2019
Merge: 6e1be6a 757581e
Author: William H.P. Nielsen <whpn@mailbox.org>

    Merge pull request #1597 from qutech-sd/bug/UHFLI-can-not-update-all-parameters
@qSaevar qSaevar deleted the bug/UHFLI-can-not-update-all-parameters branch June 19, 2019 09:14
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.

None yet

2 participants