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

Refactor Attribute/Module/Pyrpl (not ready at all) #83

Merged
merged 656 commits into from
Apr 2, 2017

Conversation

SamuelDeleglise
Copy link
Collaborator

This is not ready at all:

  • no unit testing done yet
  • never even tried on python 2
  • still needs a lot of new tests and code documentation

However, it is functional for some stuffs (on python 3) and the main structure is not expected to move a lot now. In particular, NetworkAnalyzer is the first high-level Module to be up-and-running. I am very interested by some feedback (once your schedule becomes less crazy)

Main changes:

Attributes:

  • The basic class for a field that can be get/set graphically is an Attribute (see attribute.py)
  • Obviously the Registers also inherit from Attribute (inside attributes.py)

Modules:

  • Modules are now split in HardwareModules mostly made of Registers (formerly redpitaya_modules) and SoftwareModules mostly made of basic Attributes. The aim is to make Lockbox only one SoftwareModule amongst others (NetworkAnalyzer, SpectrumAnalyzer, CurveFitter).
  • HardwareModules are only available to the user via some ModuleManagers (IqManager, AsgManager, PidManager, ScopeManager), that deal with reserving/freeing the module.

Gui:

  • Only 3 files that map 1-to-1 the code structure:
    attribute_widgets.py, module_widgets.py, pyrpl_widget.py
  • Still possible to completely disable the gui via a config file option
  • Gui is not updated by timers anymore, but by the set function of BaseAttribute.
  • Slave module (reserved through ModuleManager) have their widget disabled (+ title changed to show who owns them)
  • ModuleWidgets can now be closed and open again via a menu of the pyrpl_widget

Config file:

  • Each module instance has one top-level-entry in the config file (except for modules that have been slaved forever). This means the configuration of the lockbox signals, lock sequence... and so on will be done inside the lockbox section.
  • 2 top-level items are an exception: repitaya (ip adress, login, ...) and pyrpl (gui or not, log_level, window positions, modules to load) on this last point, users only interested in low-level modules can simply remove Lockbox and CurveFitter from this list. Also, a possibility would be to use the same option in case only the curve fitter is needed (without hardware available).
  • Attributes that are in the list module.setup_attributes are saved automagically in config files with the same technic as for gui update. For instance, at start-up, the na is in the same state it was left before. In case the user wants to start with a fresh configuration, he can simply delete the corresponding top-level entry in the config-file.
  • It's always possible to prevent some module classes from saving their state by not instantiating the setup_attribute list.

@lneuhaus
Copy link
Collaborator

The entire discussion about this pullrequest is in issue #84 (should rather have been here but now it is a little late)

# docstring does not work yet, see:
# http://stackoverflow.com/questions/37255109/python-docstring-for-descriptors
# for now there is a workaround: call Module.help(register)
class BaseRegister(object):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure we dont want from_python and to_python from intregister here?

return obj._read(addr)


class NumberRegister(BaseRegister):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the class NumberRegister even necessary? The constructor that does not make use of the bits argument may be quite misleading, or is there a reason?

class FrequencyRegister(FloatRegister, FrequencyAttribute):
"""Registers that contain a frequency as a float in units of Hz"""
# attention: no bitmask can be defined for frequencyregisters
CLOCK_FREQUENCY = 125e6
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a typical case where we might want to move the definition to a global_config.yml file, such that the code allows a clean migration to other hardware platforms

"""
widget_class = FrequencyAttributeWidget

def __init__(self, default=None, increment=0.1, min=0, max=125e6/2, doc=""):
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, the clock frequency is quite hidden here... -> move to config file?

Copy link
Collaborator

@lneuhaus lneuhaus Nov 30, 2016

Choose a reason for hiding this comment

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

especially, because the actual clock frequency is 125e6*frequency_correction


def __init__(self, default=None, increment=0.001, min=-.1, max=1., doc=""):
super(FloatAttribute, self).__init__(default=default, doc=doc)
super(FloatAttribute, self).__init__(default=default, doc=doc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this doubling desired?

@lneuhaus lneuhaus mentioned this pull request Nov 30, 2016
This was referenced Jan 16, 2017
@SamuelDeleglise
Copy link
Collaborator Author

SamuelDeleglise commented Jan 27, 2017

Oh yeah baby !!!!

unittest

Not 100% sure though what makes them sometimes fail when they are executed all together, depending on the order...

@lneuhaus
Copy link
Collaborator

Python 2.7 isnt on your side:

======================================================================
ERROR: pyrpl.test.test_attribute_widgets.TestClass.test_dummy_module

Traceback (most recent call last):
File "C:\Python27\lib\site-packages\nose\case.py", line 197, in runTest
self.test(*self.arg)
File "W:\pyrpl\pyrpl\test\test_attribute_widgets.py", line 31, in test_dummy_module
assert(isinstance(self.pyrpl.dummy_module.true_or_false, bool))
AttributeError: 'Pyrpl' object has no attribute 'dummy_module'

======================================================================
ERROR: pyrpl.test.test_hardware_modules.test_scope.TestClass.test_scope_rolling_mode_and_running_sta
te_update

Traceback (most recent call last):
File "C:\Python27\lib\site-packages\nose\case.py", line 197, in runTest
self.test(*self.arg)
File "W:\pyrpl\pyrpl\test\test_hardware_modules\test_scope.py", line 29, in test_scope_rolling_mod
e_and_running_state_update
self.r.asg1.frequency = 0
File "W:\pyrpl\pyrpl\modules.py", line 384, in setattr
super(BaseModule, self).setattr(name, value)
File "W:\pyrpl\pyrpl\attributes.py", line 62, in set
self.value_updated(instance, value) # lauch signal and update
File "W:\pyrpl\pyrpl\attributes.py", line 88, in value_updated
self.save_attribute(module, value)
File "W:\pyrpl\pyrpl\attributes.py", line 118, in save_attribute
module.c[self.name] = value
File "W:\pyrpl\pyrpl\modules.py", line 301, in c
manager_section = getattr(self.parent.c, manager_section_name)
File "W:\pyrpl\pyrpl\memory.py", line 198, in getattribute
if isbranch(self[name]):
File "W:\pyrpl\pyrpl\memory.py", line 217, in getitem
return self._data[item]
File "C:\Python27\lib\site-packages\ruamel\yaml\comments.py", line 426, in getitem
return ordereddict.getitem(self, key)
KeyError: 'asgs'

======================================================================
FAIL: pyrpl.test.test_attribute_widgets.TestClass.test_config_file

Traceback (most recent call last):
File "C:\Python27\lib\site-packages\nose\case.py", line 197, in runTest
self.test(*self.arg)
File "W:\pyrpl\pyrpl\test\test_attribute_widgets.py", line 28, in test_config_file
assert("DummyModule" in self.pyrpl.c.pyrpl.modules)
AssertionError:

@lneuhaus
Copy link
Collaborator

FAIL: pyrpl.test.test_lockbox.TestClass.test_delete_output

Traceback (most recent call last):
File "C:\Python27\lib\site-packages\nose\case.py", line 197, in runTest
self.test(*self.arg)
File "W:\pyrpl\pyrpl\test\test_lockbox.py", line 45, in test_delete_output
assert len(widget.all_sig_widget.output_widgets)==old_len-1
AssertionError:

if isinstance(module, ModuleManager):
with module.pop("foo") as mod:
assert(mod.owner=='foo')
assert(mod.owner==None)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be is

@lneuhaus
Copy link
Collaborator

lneuhaus commented Apr 2, 2017

pulling this into master now because it passes tests on travis and is more stable than original master version (release 0.9.1)

@lneuhaus lneuhaus merged commit 012ae95 into master Apr 2, 2017
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.

3 participants