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

Q: Initialization procedure #175

Open
leloup314 opened this issue Mar 7, 2022 · 10 comments
Open

Q: Initialization procedure #175

leloup314 opened this issue Mar 7, 2022 · 10 comments

Comments

@leloup314
Copy link
Member

leloup314 commented Mar 7, 2022

Is there a particular reason for the following syntax to be necessary?

from basil.dut import Dut

# Instantiate my_device
my_device = Dut(my_config)
my_device.init()  # Additionally call an init method on the instance manually

Why not call my_device.init inside the instances __init__?

@themperek
Copy link
Member

I do not remember anymore but there was a reason for this.
We sometimes manipulate things before actually connecting to hardware.

@leloup314
Copy link
Member Author

From my subjective perspective it looks like 90% of the use cases do not need to do this.
Maybe it would be nice to have something like a default argument Dut(auto_init=True) that automatically calls the init as last action inside the __init__ if set. If not wanted, set auto_init=False

@themperek
Copy link
Member

Dut(auto_init=False)

This should be OK.

@leloup314
Copy link
Member Author

Well this will not change anything for anyone ;) I would add Dut(auto_init=True) and issue a DeprecationWarning when people do

my_device = Dut(my_confif, auto_init=True)
my_device.init() 
# Prints DeprecationWarning("Manual call to 'init'-method is deprecated if 'auto_init=True'. Use 'auto_init=False' if manual 'init'-call is required" )

This way change could actually happen. I can implement this easily if you want to test

@leloup314
Copy link
Member Author

But of course this would break some stuff. My point is to design the API for the edge-cases is not the right approach. Can you tell me where you do something in between the initailization of the Dut and the init call?

@themperek
Copy link
Member

In general OK but not sure if this is a good idea to do now. Should accumulate more braking changes?

@themperek
Copy link
Member

@leloup314
Copy link
Member Author

The only line where something happens in between initialization and manual call to init is for the clock generator. Everything else looks fine to me.

@leloup314
Copy link
Member Author

But yes, maybe we can wait and have a more thorough look at this, maybe accumulate other braking changes.

@cbespin
Copy link
Contributor

cbespin commented Mar 10, 2022

Maybe not best example: https://gitlab.cern.ch/silab/bdaq53/-/blob/development/bdaq53/system/bdaq53.py

Just to add, since tj-monopix2-daq is almost exactly like bdaq, this will also be affected. No problem for me to adapt, though.

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

No branches or pull requests

3 participants