Skip to content

Conversation

@Narg3000
Copy link

@Narg3000 Narg3000 commented Jul 5, 2022

Astropy is a module which simplifies working with and programing the sensor.

This is not ready to be merged since it hasn't been fully tested on known good hardware yet and there are still some files which need removed/changed.

beam_test.py Outdated
if i >= args.maxhits: break

if astro.hits_present(): # Checks if hits are present
time.sleep(.1) # this is probably not needed, will ask Nick
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 needed?

@c235labtop
Copy link

c235labtop commented Oct 7, 2022

[AS] @nic-str I (Amanda) have incorporated all of these updates to this branch. Please look over it when you get a chance and hopefully early next week (and before you incorporate large changes again) we can finally merge.

Enhancements to consider AFTER merge:

  • remove methods duplicated in astropix.py and asic.py
  • additional dictionary in yml for voltageboard dacs

@nic-str
Copy link
Owner

nic-str commented Oct 13, 2022

Do we still need these separate mask files with the yaml config files?

Also there are a lot of scripts included as well as instrument control classes, which are not really related to the astropy module.

Then there is still the issue with a lot of duplicate functions, copied from other modules, in the astropy class. Can this be solved?

I would also like to create a separate folder for astropy and call it e.g. "chips/astropy2.py", because it's really specific to v2. Then we could rename the modules folder to something like "core". The modules folder could then be used for extensions like the hitplotter, which are cool but not always necessarly needed.

@c235labtop
Copy link

(from Amanda)
@nic-str - I addressed most of your comments in these commits.

I left the python run scripts because I intended for them to be examples from which people could branch off and write their own. I considered moving them to an exampleScript directory but didn't want to deal with updating paths and trying to account for whether the operator was using Mac/OS/Linux

I also didn't add a greater chip directory because I'm confused about what would go here. Just the astropix.py module? That module and my example scripts? What big changes are forseen between v2 and v3 that the module couldn't handle?

@asteinhebel
Copy link

@nic-str Just checking in on the status of this request. Are we close to merging? The final action is yours

@asteinhebel
Copy link

@nic-str Can we take any motion on this merge? If you prefer and think that it is mature enough, we can migrate this code to the shared AstroPix collaboration space and begin developing the shared code base instead of fully merging here. Then we would all utilize / develop in the AstroPix space

gs66c235 added 4 commits December 12, 2022 13:10
… with AS optimization illustrated in comments. config_c0_r0 activates pixel c0r0 and utilizes all optimized DACs identified by AS
… over full array. Update astropix.py to allow injection enabling outside of asic_init [AS[
…ip with beam_test.py. Will need updates to other example scripts to match path structure and check advanced functionality but baseline running works
@c235labtop
Copy link

[Amanda] @nic-str I have incorporated your updates so we should be good to go! I am content now to merge and migrate if you are willing to initiate by approving the merge.

I think there is still some cleaning to be done in the collaboration space but since I don't fully understand the structure there yet it may be easiest to merge and migrate and then file paths can be updated.

A reminder that I will be on holiday until Dec. 26 and then infrequently at work until Jan. 3. I would like to have the collaboration space finalized and ready to share with our collaborators by Jan. 16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants