Closed riedel-ferringer closed 2 years ago
JimLewis
commented
2 years ago This is done by LocalScriptDefaults.tcl right? See end of #25
I note in the file that you used as an example that you have numerous revisions to things like build - to catch errors and such. I am certainly interested in things like that.
riedel-ferringer
commented
2 years ago This is done by LocalScriptDefaults.tcl right? See end of #25
Right. I will change that. Wasn't aware of that file.
Yes I am catching errors, because I need to know if a command fails to correctly terminate database entries. It's just something I need and probably not necessary for the main repo to support.
The callbacks on the other hand I think are really handy. Because it gives you a lot of flexibility and you don't have to change the scripts (as they are a nightmare to merge). Clearly, one would have to think about usefull callbacks and their params. My suggestions are incomplete and just tailored to my current requirements.
riedel-ferringer
commented
2 years ago On the other hand - if we want to include callbacks and probably also "internal" versions of the commands for error-catching - a separate file which already contains all the supported callbacks and interface functions (just like my CustomScripts.tcl but without any implementation bodies) would be preferrable. One could just add his implementation without much research.
riedel-ferringer
commented
2 years ago I pushed a first proposal for the callback functionality. I think this structure is the most flexible, and it should be quite easy to understand and use from an enduser's point of view. Some Remarks:
I am looking forward to reading your feedback.
JimLewis
commented
2 years ago If we follow the pattern vendor_
The file LocalScriptDefaults.tcl is reserved for user interaction with the scripting environment and cannot exist in the OSVVM repository as it would overwrite their file when they update.
We could have a CallBacks_default.tcl. I would prefer that the default callbacks do nothing (ie: no printing).
When a build fails we need to return an exit code for the CI tool so that it knows the test failed. OTOH, within library, analyze, and simulate, we want to allow all simulations that can run to run - but if something failed, we need to remember that and report it when build finishes. So we need some sort of error flag that is reset when build starts and is checked when build finishes.
As we start to do more overrides, we need to be careful about the call order of the scripts so that there is a precedence between the Osvvm settings, the vendor settings, and the user local settings.
I have a tactical bug fix in simulate that will impact things. It looks like vendor_simulate needs to know the test case name and that maybe it should be passed. Currently the code coverage is named after the library unit, but unless someone is using configurations, the library unit name may be the top level testbench name rather than the test case name.
Update: I see in the script that TestScriptName variable is referenced and used, so I will not update the call interface, but instead use the TestCaseName variable
JimLewis
commented
2 years ago I like the concept of your idea about LocalScriptDefaults.tcl - I just don't want to name it that as I want newbies of git to be able to do this. So how about TemplateLocalScriptDefaults.tcl for the local script settings. I think it should have in it a parsing for the simulator to call the appropriate CallBacks
riedel-ferringer
commented
2 years ago I like the concept of your idea about LocalScriptDefaults.tcl - I just don't want to name it that as I want newbies of git to be able to do this. So how about TemplateLocalScriptDefaults.tcl for the local script settings. I think it should have in it a parsing for the simulator to call the appropriate CallBacks.tcl.
The reason I put it in LocalScriptDefaults.tcl (I know it shouldn't be checked in, but I needed it for demonstration purposes) is that it can be totally user-defined. It doesn't need to exist. So there wouldn't be the need for a Template_...tcl file. If someone wants to use an event handler, he can just add it to the localscripts file. Which handlers exist must of course be documented. I didn't intend to provide empty dummy handlers.
When a build fails we need to return an exit code for the CI
This seems tricky, because when do you reset this flag? The problem is that "build" can be called recursively / nested. And resetting it in StartUp.tcl will not work, because it's only called once per session (if in GUI).
If we follow the pattern vendor_.tcl,
So if I understand you correctly, you want separate callbacks for each simulator/vendor. While that's fine, I am wondering if there are people out there who really use multiple simulators simultaneously?
... we don't need ExecuteIfExists
I think this is only partly true. Although we don't need this specific function, we need some other "executor function", because we need to catch any errors that occur in the user defined callbacks.
JimLewis
commented
2 years ago WRT Build, build is only intended to be called from one spot - most often this is the tool command line. Build establishes a reporting layer. Internally there is also include (which build calls to do everything other than the reporting layer).
WRT more than one simulator, I expect this to become very common - ie GHDL for batch and regression and a commercial simulator for debug. Nonetheless, having a naming pattern that supports this allows users to do what they need.
I realize I was the one who wanted the injection of the callbacks to be in the outer layer if possible. However as I start to think about it, I am not sure that is practical for some simulators - like Cadence Xcelium and Synopsys VCS - the simulation is done when simulate completes - hence I don't think anything we do there is going to be meaningful in a callback in the outer layer - however in the VendorScripts_XXX.tcl I suspect we may be able to do something meaningful. I need to experiment some.
riedel-ferringer
commented
2 years ago Should I prepare another proposal with a naming scheme like CustomScripts_Mentor.tcl, CustomScripts_GHDL.tcl etc and add the callbacks to all the VendorScript_xxx.tcl files? To start with, I would just add it to Mentor for review. If we can agree to the style, I would extend it to the other vendors
JimLewis
commented
2 years ago I have implemented the error handling already following the pattern you suggested. This is in dev.
The call backs are still on the todo list. I think the call backs for everything except simulate can be in OsvvmProjectScripts, however, for simulate, I think it has to be in the context of the run portion of the scripts.
JimLewis
commented
2 years ago @riedel-ferringer I just pushed an update to the dev branch with trial implementation for callbacks.
Let me know if this does what it needs to do. It resulted general clean up of the error handling - so that was good.
JimLewis
commented
2 years ago OOPS. I pushed that to main and not dev. So it is out there for anyone who pulls it.
I propose to make the scripts easy to customize.
I propose a separate file, CustomScripts.tcl, which contains callbacks and the like. This way, customers can easily add functionality without messing around in the original code. Continuous merges with the github repo are thus easy. The contents of the current CustomScripts.tcl is of course my own implementation, and I just added it for you to get a feeling about the desired functionality (and that you have a working implementation to play around with),
Please notice that this is my current "minimally invasive" implementation, so that future merges are easy. Should you consider this PR, I can integrate it more tightly and more complete. I just added the callbacks I need - for a release, many more callbacks should be added.