pcdshub / pytmc

Generate EPICS IOCs and records from TwinCAT projects - along with many TwinCAT project tools
https://pcdshub.github.io/pytmc/
Other
10 stars 11 forks source link

REF: sort record fields #136

Closed klauer closed 4 years ago

klauer commented 4 years ago

This doesn't have to be merged - but we should decide sooner rather than later if we are going this route.

Closes #109

Pluses:

Minus:

If we think the plus outweighs the minus, then let's go with it. If not, we can close this.

Example before:

record(mbbi, "MR1K4_GCC_1:STATE_RBV"){
  field(PINI, "1")
  field(TSE, "-2")
  field(ZRVL, "1")
  field(ZRST, "PressInvalid")
  field(ONVL, "2")
  field(ONST, "GaugeDisconnected")
  field(TWVL, "3")
  field(TWST, "Off")
  field(THVL, "4")
  field(THST, "Starting")
  field(FRVL, "6")
  field(FRST, "OoR")
  field(FVVL, "10")
  field(FVST, "Valid")
  field(SXVL, "11")
  field(SXST, "ValidHi")
  field(SVVL, "12")
  field(SVST, "ValidLo")
  field(INP, "@asyn($(PORT),0,1)ADSPORT=851/GVL_DEVICES.MR1K4_GCC_1.IG.eState?")
  field(DTYP, "asynInt32")
  field(SCAN, "I/O Intr")
}

After:

record(mbbi, "MR1K4_GCC_1:STATE_RBV"){
  field(DTYP, "asynInt32")
  field(FRST, "OoR")
  field(FRVL, "6")
  field(FVST, "Valid")
  field(FVVL, "10")
  field(INP, "@asyn($(PORT),0,1)ADSPORT=851/GVL_DEVICES.MR1K4_GCC_1.IG.eState?")
  field(ONST, "GaugeDisconnected")
  field(ONVL, "2")
  field(PINI, "1")
  field(SCAN, "I/O Intr")
  field(SVST, "ValidLo")
  field(SVVL, "12")
  field(SXST, "ValidHi")
  field(SXVL, "11")
  field(THST, "Starting")
  field(THVL, "4")
  field(TSE, "-2")
  field(TWST, "Off")
  field(TWVL, "3")
  field(ZRST, "PressInvalid")
  field(ZRVL, "1")
}
codecov-io commented 4 years ago

Codecov Report

Merging #136 into master will decrease coverage by 0.1%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
- Coverage   78.79%   78.68%   -0.11%     
==========================================
  Files          16       16              
  Lines        1820     1820              
==========================================
- Hits         1434     1432       -2     
- Misses        386      388       +2
Impacted Files Coverage Δ
pytmc/linter.py 68.25% <0%> (-3.18%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1ce15be...a6226e9. Read the comment docs.

ZLLentz commented 4 years ago

We definitely need the fields to be written in a deterministic order. I'm already running into fields that get ordered differently at different times in ways that I'm unable to diagnose.

Alphabetical has the disadvantages you've noted but is clearly the easiest to implement (you've achieved this in a 5 character diff).

Unless there's an easy way to categorize fields and have a "natural-feeling" grouping without going through one-by-one and defining a static order, I think this should be merged.

klauer commented 4 years ago

Without doing it manually, my only other thought is that we could use something like softIoc.dbd to generate per-record field ordering for us. Then the jinja filter would use that as the sort(key=...)

ZLLentz commented 4 years ago

This would work as long as the dbd files are always built with consistent ordering and if there's an easy way to extract it

n-wbrown commented 4 years ago

I've created #143 which sorts recognized entries in a coherent way and alphabetizes the rest.