search

areaDetector / ADCore

The home of the core components of the EPICS areaDetector software. It includes base classes for drivers and code for all of the standard plugins.
https://areadetector.github.io/areaDetector
Other
20 stars 69 forks source link

Remove obsolete edl files #310

Open MarkRivers opened 6 years ago

MarkRivers commented 6 years ago

Many of the files in ADApp/op/edl are probably not needed and should be removed. The adl files are can now be automatically converted to edl, ui, and opi by running make in the op/ directory. The edl/autoconvert files are thus in many cases more up to data than the equivalent file in edl/, and the ones in edl/ should thus be removed. I am not an edm expert, and I don't know what files are in use at edm sites (e.g. SLAC and Diamond), so I would like people from those sites to delete the obsolete files.

ulrikpedersen commented 6 years ago

I think most of those EDM files in edl/ are used by the "DLS screens" (i.e. the 'tabbed' EDM screen layout that @thomascobb did years ago). We do maintain those screens although they rarely change. We can review which ones are used and which can go.

ulrikpedersen commented 6 years ago

@MJGaughran will take a look at this to identify what we use and what we don't use at DLS :-)

bhill-slac commented 6 years ago

I've been trying to resolve how to deal w/ edl file changes here at SLAC. We started w/ the screens now in ADApp/op/edl years ago and have made many changes since then to make them more consistent w/ our fonts and colors, fix layout issues, etc. I've hesitated to submit these changes, as one change in particular as it isn't backward compatible. We prefix each embedded or related screen reference w/ "areaDetectorScreens/", which allows us to use soft links to locate the desired release of the screens. The workaround isn't hard, it just requires these edm screens to be deployed in a directory named areaDetectorScreens, or to add a soft link w/ that name in your EDMDATAFILES path. I've just in the last few days collected all our edl changes as a git branch which starts w/ a recent copy of ADApp/op/edl/*.edl in ADApp/op/edl/slac-edl and applies our changes as a series of commits.

I'm thinking the best way forward would be for me to submit this branch as a pull request. Then @MJGaughran can give them a try. I'll also take a look myself at whether or not there are any obsolete files. Then we can decide whether or not to keep the slac-edl variant or merge them w/ the current set. Cheers, Bruce

ulrikpedersen commented 6 years ago

I'm also aware that the "DLS" style of EDM screens may not work easily for others as we auto-generate the top-level "tabbed" container screen and also have a 'standard' colour-scheme. So now we have up to 3 sets of somewhat different screens it may be time to separate them in an organised fashion?

In my view there are a couple of ways to do this:

Thoughts?

bhill-slac commented 6 years ago

I've reviewed the edl screens and as far as I can tell NDFileCBF.edl and NDFileReduced.edl are obsolete.

They both seem to be embedded panels that are no longer used.

Looking in autoconvert, I see a lot of screens that aren't in edl or slac-edl. I haven't had a chance yet to see if any of those screens would be useful or if any are obsolete.

Re SLAC vs DLS vs autoconvert, I've just submitted a pull request that adds the slac edl files in ADApp/op/edl/slac-edl.    Some of the commits are ones you might be interested in.

e3ecee6 Add optional display of position or center for overlays cabe660 Add title line for each plugin popup screen 90e18a3 Reworked layout of NDFile panel using visibility groups for different file capture modes.

These are based on the most recent files in ADApp/op/edl so you could probably apply them by creating a patch file and applying it w/ the appropriate strip level.

Cheers,

On 02/01/2018 02:26 AM, Ulrik Kofoed Pedersen wrote:

I'm also aware that the "DLS" style of EDM screens may not work easily for others as we auto-generate the top-level "tabbed" container screen and also have a 'standard' colour-scheme. So now we have up to 3 sets of somewhat different screens it may be time to separate them in an organised fashion?

In my view there are a couple of ways to do this:

  • Move all site-specific screens out into separate repositories. Keeping only the auto-converted (and tweaked) EDM screens from @MarkRivers https://github.com/markrivers in ADCore
  • Move each site-specific set of screens into separate dirs within ADCore. I.e. op/edl/dls nad op/edl/slac

Thoughts?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/areaDetector/ADCore/issues/310#issuecomment-362223487, or mute the thread https://github.com/notifications/unsubscribe-auth/AF43JjKXXXpuRsuNZXmzaSWGFKAxrF0yks5tQZFegaJpZM4RvyP8.

-- Bruce Hill Member Technical Staff SLAC National Accelerator Lab 2575 Sand Hill Road M/S 10 Menlo Park, CA 94025

MJGaughran commented 6 years ago

We agree with the removal of NDFileCBF and NDFileReduced. Ulrik suggests that simTop is another historical file that can be removed.

NDAttribute(N) and NDROIStat(N) are screens that appear to have never been linked to in their template files, and have a couple of mistakes. I am waiting for someone to let me know if they intend to use these screens; otherwise, they will also be deleted.

I will make a pull request once we have agreed on the edl folder structure, and I have heard back about the above screens.

MJGaughran commented 6 years ago

NDAttribute and NDROIStat screens are used in a support module, but should be maintained in the ADCore module. I may also make a couple of small changes to them.

The only thing left is to decide on the edl folder structure (separate repositories or separate folders). Any further comments on this?

bhill-slac commented 6 years ago

I'm in favor of keeping the edl sceens in ADCore as we deploy them based on the ADCore release tag. That way as new features are added to ADCore, the ioc's that use the new ADCore version have access to the corresponding screens that expose the new features.

MarkRivers commented 6 years ago

Do you also plan to have a slac-edl directory for each detector? I am ambivalent about putting site-specific files in the areaDetector respository.

bhill-slac commented 6 years ago

I think ADCore is the most important set of screens to make available because of all the plugins.

The driver screens don't change often as they're typically either an embedded panel w/ the most important driver specific features or separate screens to access the full feature set or help.

I'm ambivalent about the directory names and organization but think it doesn't hurt to have some alternative sets of edm screens.

On 02/06/2018 10:15 AM, Mark Rivers wrote:

Do you also plan to have a slac-edl directory for each detector? I am ambivalent about putting site-specific files in the areaDetector respository.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/areaDetector/ADCore/issues/310#issuecomment-363514751, or mute the thread https://github.com/notifications/unsubscribe-auth/AF43Jrycjy-7JvTbwCJwBXlRDTonPiKRks5tSJbYgaJpZM4RvyP8.

-- Bruce Hill Member Technical Staff SLAC National Accelerator Lab 2575 Sand Hill Road M/S 10 Menlo Park, CA 94025

ulrikpedersen commented 6 years ago

For us it would definitely be easier to keep the EDM screens in ADCore, as long as they don't get mangled up and conflict with other sites screens ;-). However, in my opinion and from a (areaDetector) maintainer point of view, the site-specific screens really don't belong in ADCore. So on the whole I'm in favour of moving these screens out of ADCore as The Right Thing To Do(tm).

There are a couple of ways that sites like DLS or SLAC can handle this with minimal fuss:

bhill-slac commented 6 years ago

Re a fork w/ a site-specific branch, we're already doing that w/ the slac-edl branch on bhill-slac/ADCore.

Mark, do you know of any other sites that are using edm screens for ADCore, and if so, are they using ADApp/op/edl/*.edl or the autoconvert screens?

I see issues w/ both the existing sets which are why we're maintaining a 3rd set.

Ulrik, I don't see the top-level "tabbed" container screens that you mentioned. Are those generated or maintained outside ADCore?

DLS screens in ADApp/op/edl:

autoconvert screens:

slac-edl screens:

Screenshot from autoconvert: adapp-op-edl-autoconvert-ndfiletiff

Screenshot from ADApp/op/edl/NDFileTIFF.edl: adapp-op-edl-ndfiletiff

Screenshot from slac-edl/NDFileTIFF.edl adapp-op-edl-slac-edl-ndfiletiff

MarkRivers commented 6 years ago

I would like to suggest that it might be worthwhile to spend some effort on improving the adl2edl converter. In the end if the autoconverted screens can be used directly it can save a lot of effort in manually fixing the edl files every time the adl files change because of new plugin or driver features.

To show that I think this should be feasible I show below the NDPluginStats screen from medm, and the autoconverted versions for CSS-BOY and caQtDM.

This is the medm version. ndstats_medm

This is the CSS-BOY version (with a 1 character manual fix. Kay has probably already fixed the converter for this). ndstats_css

This is the caQTDM version. ndstats_caqtdm

I think you will have to admit that these conversions are very good. I am sure that the edm converter could be made to be just as good.

But here is the autoconverted edm file. I am not an edm expert at all, so there could be things that are not configured correctly in my edm environment.

It looks like the problems are mostly with colors and fonts, which should not be that hard. The related display buttons don't work at all for me.

ndstats_edm

bhill-slac commented 6 years ago

I've been looking into improving adl2edl and have a number of fixes that can greatly improve the autoconverted screens. I'll be pushing those up to epicsdeb/adl2edl.git as I finish them. The font matching code isn't working at all which is why the fonts look so bad above. Changing the default fonts to more reasonable defaults helps a lot. I'll look further into why font matching isn't working. There's a -rgb flag that can be used to get the same colors as the medm screen. When -rgb isn't used, the current version just stuffs in the medm color number which isn't useful as the color tables aren't very similar resulting in unreadable text. Introducing default color numbers for key widgets makes the screens look a lot more like the DLS and SLAC color scheme. The resulting screens are much better, but we'll still use our versions as some hand tweaking is needed for most files, and we've made other changes and improvements. Mark, I'll leave it up to you whether or not to include the slac-edl screens. If you do I'll push updates as I make them. I think DLS should be the only site to update the ADApp/op/edl/*.edl files since it's difficult to merge edm files after they've been edited as the widget order in the file can easily change.

MJGaughran commented 6 years ago

Is there any information on what other sites are using the DLS screens?

Would it be best to wait for the full update to adl2edl, and then remove all of the DLS screens from this repository and keep them on our own branch?