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 clocks and powersupply #4
Conversation
TCD1304.py
Outdated
return analog_measurement # this should put the Measurements and timestamps in an array | ||
icg_clock() | ||
x, = self.scope.capture() # putting timestamps into numpy array (don't know, if that is necessary for our purpose) | ||
y, = self.scope.fetch_data() # collecting the Voltage in nonblocking mode |
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.
You probably want to return this.
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.
not needed: x, = self.scope.capture()
y, = self.scope.fetch_data() # collecting the Voltage in nonblocking mode
return [y]; #returns samples
Correct?
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.
You could just return y
as is, no need to put it in a list.
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.
fair point
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.
I corrected the "last" few things and committed it.
Are there any more errors to fix?
How do we actually test the code? Freddi and myself are planning to write the report for the upcoming tuesday which means, it would be super cool to have some sort of a "goodie" for our professor.
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.
The first step to test the code is to check that you get some kind of output at all from the sensor. You have the sensor set up, right? What happens when you connect it to the PSLab and run this code?
If you get some output, the next step is to compare it to a known reference. A fluorescent light source with a known spectrum, for example. The output should match the reference.
As for the code, there are still a few errors (which I will address separately). Aside from the errors, there are some more things to do before the code can be merged:
- Usability: Consider how a user might be expected to use this module. What do they want to do? Is the code written in a way to make the user's task as simple as possible? For example, in order to get some output from the sensor, it is currently necessary to do something like this:
from TCD1304 import TCD1304
spectrometer = TCD1304()
spectrometer.power_source()
spectrometer.master_clock()
spectrometer.sh_clock()
output = spectrometer.analog_signal_read()
Is there any reason why the user should have to manually power the device and set up the clocks? Why not start them automatically when the object is created? It would be more user friendly like this:
from TCD1304 import TCD1304
spectrometer = TCD1304()
output = spectrometer.read()
- Various style issues:
- Module- and class-level constants should be capitalized.
- Function names should be verbs (e.g.
start_sh_clock
rather thansh_clock
) - Function names should be as short and simple as possible while still being descriptive. In the method
analog_signal_read
there is no need to specify "analog" since the sensor cannot do a digital read. The name could be justread
ormeasure
.
- Documentation:
- The module, class, and methods need docstrings explaining what they do. You currently have a lot of inline comments that could be moved to docstrings. Docstrings should be written in numpydoc style.
TCD1304.py
Outdated
return analog_measurement # this should put the Measurements and timestamps in an array | ||
icg_clock() | ||
x, = self.scope.capture() # putting timestamps into numpy array (don't know, if that is necessary for our purpose) | ||
y, = self.scope.fetch_data() # collecting the Voltage in nonblocking mode |
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.
The first step to test the code is to check that you get some kind of output at all from the sensor. You have the sensor set up, right? What happens when you connect it to the PSLab and run this code?
If you get some output, the next step is to compare it to a known reference. A fluorescent light source with a known spectrum, for example. The output should match the reference.
As for the code, there are still a few errors (which I will address separately). Aside from the errors, there are some more things to do before the code can be merged:
- Usability: Consider how a user might be expected to use this module. What do they want to do? Is the code written in a way to make the user's task as simple as possible? For example, in order to get some output from the sensor, it is currently necessary to do something like this:
from TCD1304 import TCD1304
spectrometer = TCD1304()
spectrometer.power_source()
spectrometer.master_clock()
spectrometer.sh_clock()
output = spectrometer.analog_signal_read()
Is there any reason why the user should have to manually power the device and set up the clocks? Why not start them automatically when the object is created? It would be more user friendly like this:
from TCD1304 import TCD1304
spectrometer = TCD1304()
output = spectrometer.read()
- Various style issues:
- Module- and class-level constants should be capitalized.
- Function names should be verbs (e.g.
start_sh_clock
rather thansh_clock
) - Function names should be as short and simple as possible while still being descriptive. In the method
analog_signal_read
there is no need to specify "analog" since the sensor cannot do a digital read. The name could be justread
ormeasure
.
- Documentation:
- The module, class, and methods need docstrings explaining what they do. You currently have a lot of inline comments that could be moved to docstrings. Docstrings should be written in numpydoc style.
TCD1304.py
Outdated
self.ps = PowerSupply() | ||
|
||
def power_source (): # puts a voltage of 4V on PV1 | ||
ps = PowerSupply() |
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.
You should use the self.ps
object instead of creating a new one here.
TCD1304.py
Outdated
def power_source (): # puts a voltage of 4V on PV1 | ||
ps = PowerSupply() | ||
ps.pv1 = 4 | ||
ps.pv1 |
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.
As pointed out before, this line is unnecessary.
TCD1304.py
Outdated
self.scope.capture(channels=1, samples=integration_elements*samples_per_element, timegap=integration_time/samples_per_element, block=False) | ||
self.icg_clock() | ||
y, = self.scope.fetch_data() # collecting the Voltage in nonblocking mode | ||
return y; #returns samples |
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.
No ; needed.
TCD1304.py
Outdated
ps.pv1 = 4 | ||
ps.pv1 | ||
|
||
def icg_clock (self): # on SQ3 starts and stops the reading of the sensor; this third clock is the slowest clock |
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.
Don't put whitespace between the function name and the parentheses.
TCD1304.py
Outdated
samples_per_element = 2 | ||
read_out_time = integration_time*integration_elements | ||
|
||
class TCD1304(): |
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.
No parentheses needed.
TCD1304.py
Outdated
def sh_clock (self): # on SQ2 sets pulse for the integrationtime. Running the Sensor in shutter mode try with tint (min) = 10µs | ||
self.pwmgen.generate(["SQ2"], 1/integration_time, [0.5] ) | ||
|
||
def analog_signal_read (self, block:False): |
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.
This method should not take a block
argument. The argument syntax is also incorrect.
self.scope = Oscilloscope () | ||
self.ps = PowerSupply() | ||
|
||
def set_power_source(): |
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.
Needs self
argument in definition.
No description provided.