Skip to content

Conversation

@Araneidae
Copy link
Collaborator

This commit addresses both github issues #81 and #67.

The arguments to the a and long record builder functions (aIn, aOut,
longIn, longOut) are brought more into line with the arguments as
used in the epics_device module. This means that extra unnamed arguments
can be used to specify EGU and PREC (where appropriate), and for out
records the DRV limits take precedence.

As a side effect, the implementation of this is made closer to that in
https://github.com/dls-controls/epics_device/

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #82 (3a5586e) into master (0ace830) will increase coverage by 0.23%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
+ Coverage   87.36%   87.59%   +0.23%     
==========================================
  Files          13       13              
  Lines         910      927      +17     
==========================================
+ Hits          795      812      +17     
  Misses        115      115              
Impacted Files Coverage Δ
softioc/builder.py 97.40% <100.00%> (+0.32%) ⬆️

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 0ace830...3a5586e. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Feb 16, 2022

Unit Test Results

     12 files  ±  0       12 suites  ±0   15m 24s ⏱️ +27s
   224 tests +  4     218 ✔️ +  4      6 💤 ±0  0 ±0 
2 688 runs  +48  2 340 ✔️ +48  348 💤 ±0  0 ±0 

Results for commit 3a5586e. ± Comparison against base commit 0ace830.

♻️ This comment has been updated with latest results.

This commit addresses both github issues #81 and #67.

The arguments to the a and long record builder functions (`aIn`, `aOut`,
`longIn`, `longOut`) are brought more into line with the arguments as
used in the epics_device module.  This means that extra unnamed arguments
can be used to specify EGU and PREC (where appropriate), and for out
records the DRV limits take precedence.

As a side effect, the implementation of this is made closer to that in
https://github.com/dls-controls/epics_device/
Previously all "in" records were filtered out of this test
Copy link
Collaborator

@AlexanderWells-diamond AlexanderWells-diamond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes seem good to me. Just a couple of clarifying questions.

There's no new tests, so I don't believe anything is currently exercising the _set_scalar_out_defaults function fully. It'd be good to see at least a trivial test that tries to set different DRVL/H and L/HOPR values and confirm the record is as expected. I don't know if there's any situations that might give invalid values (e.g. someone sets LOPR and HOPR the wrong way round) that may also warrant testing?

@Araneidae
Copy link
Collaborator Author

Regarding the lack of tests, I agree that it would be good to add some more tests.

Araneidae and others added 2 commits February 17, 2022 08:46
Also fix missing internal changelog links
@Araneidae Araneidae merged commit 3a5586e into master Feb 17, 2022
@Araneidae Araneidae deleted the issue-81 branch February 17, 2022 09:33
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

Successfully merging this pull request may close these issues.

Inconsistencies around defaults for Analog and Long records PINI flag is not set on In records when .set() is used

2 participants