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.

Sign up for GitHub

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

Jump to bottom

Backport monitor #731

Merged
merged 15 commits into from
Oct 16, 2017
Merged

Backport monitor #731

merged 15 commits into from
Oct 16, 2017

Conversation

jenshnielsen
Copy link
Collaborator

Backport Elm based monitor from QCoDeS DK

@jana-d

giulioungaretti and others added 8 commits September 11, 2017 12:20
@giulioungaretti @jenshnielsen
Feature/monitor (#15)
321d39d 
* First monitor commit, now 🍔 !

* polish things up

* types for the masses

* chore: Refactor monitor

* fix: Add todo remove space

* refactor

* silm api

* better monitor

* fix: Add static files to pacakge setup

* fix: Add websocket dependency

* fix: Use http with webbrowser open
@giulioungaretti @jenshnielsen
fix: Include static, add init
851a45e
@giulioungaretti @jenshnielsen
move server to onw scripts
f81fd2f
@giulioungaretti @jenshnielsen
feat: make monitor server standalone
7138f07
@giulioungaretti @jenshnielsen
fix: Sleep before thread starts!
0184668
@giulioungaretti @jenshnielsen
fix: Update monitor GUI
570fddb
@jenshnielsen
Fix monitor for channel parameters (#61)
dc52646
@jenshnielsen
The monitor does not work correctly with ws34
8e61060
jenshnielsen
jenshnielsen commented Sep 11, 2017
View reviewed changes
qcodes/__init__.py Outdated
@@ -32,6 +32,7 @@
from qcodes.loops import Loop, active_loop, active_data_set
from qcodes.measure import Measure
from qcodes.actions import Task, Wait, BreakIf
from qcodes.monitor.monitor import Monitor
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We might want to drop this import to not make websockets a hard dependency of qcodes?

@jenshnielsen
make websockets a soft dependency
ae8403c 
Everything but the monitor can be used without it
@peendebak
Copy link
Contributor

@jenshnielsen I just tested the PR from spyder. My system freezes after trying to start a monitor. I tried:

In [4]: m=qcodes.Monitor( gates.B0, gates.B1)

This results in the following error and my system being locked.

ERROR:asyncio:Exception in callback Monitor._create_task(<Future at 0x...state=pending>, <websockets.p...00211FE148AC8>)()
handle: <Handle Monitor._create_task(<Future at 0x...state=pending>, <websockets.p...00211FE148AC8>)()>
Traceback (most recent call last):
  File "C:\Users\eendebakpt\AppData\Local\Continuum\Anaconda3\envs\py35\lib\asyncio\events.py", line 125, in _run
    self._callback(*self._args)
  File "c:\svn\qcodes\qcodes\monitor\monitor.py", line 172, in _create_task
    task = self.loop.create_task(coro)
  File "C:\Users\eendebakpt\AppData\Local\Continuum\Anaconda3\envs\py35\lib\asyncio\base_events.py", line 261, in create_task
    task = tasks.Task(coro, loop=self)
  File "C:\Users\eendebakpt\AppData\Local\Continuum\Anaconda3\envs\py35\lib\asyncio\tasks.py", line 70, in __init__
    assert coroutines.iscoroutine(coro), repr(coro)
AssertionError: <websockets.py35.server.Serve object at 0x00000211FE148AC8>

@jenshnielsen
Copy link
Collaborator Author

I think this breaks with websockets 3.4 which is why setup.py limits it to 3.3. I initially missed that commit when backporting. Can you try with 3.3?

@peendebak
Copy link
Contributor

@jenshnielsen Thanks, using websockets 3.3 solved the issue. Will setting of parameters also be made an option? That would be important for us to make it more usefull.

@jenshnielsen
Copy link
Collaborator Author

It's an old monitor that we have used and found useful in Copenhagen for some time which others have been interested in using hence this pull request. It's not meant for setting. We are aware of that request and it will happen eventually

@jenshnielsen
Copy link
Collaborator Author

@WilliamHPNielsen added a readme with a link to the ELM repo

WilliamHPNielsen
WilliamHPNielsen approved these changes Oct 10, 2017
View reviewed changes
Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen left a comment

Choose a reason for hiding this comment

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

I approve of this backport. We could consider expanding the documentation, but I wouldn't give that very high priority.

@WilliamHPNielsen WilliamHPNielsen merged commit c036866 into microsoft:master Oct 16, 2017
giulioungaretti pushed a commit that referenced this pull request Oct 16, 2017
Generated gh-pages for commit c036866
ad000f7 
Author: Jens Hedegaard Nielsen <[email protected]>

    Backport monitor (#731)
@jenshnielsen jenshnielsen deleted the backport_monitor branch October 16, 2017 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WilliamHPNielsen WilliamHPNielsen WilliamHPNielsen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jenshnielsen @peendebak @WilliamHPNielsen @giulioungaretti