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

Only use the calculated length if NELM was NOT provided during record creation #38

Closed
wants to merge 4 commits into from

Conversation

AlexanderWells-diamond
Copy link
Collaborator

Closes #37

This PR is what I think the minimum fix is for the linked issue - it allows setting both initial_value and NELM during waveform record creation such that both keywords are respected.

It does not currently attempt to do anything regarding the "duplicate" length vs NELM or value vs initial_value keyword arguments. Using "mixed pairs" of Python and EPICS keywords will not work in all cases. Please let me know what, if anything, I should do with that.

A single test is written to cover the one case I originally identified - depending on the outcome of discussion I can write more, as appropriate.

@codecov
Copy link

codecov bot commented Sep 15, 2021

Codecov Report

Merging #38 (146f350) into master (ea1b85a) will increase coverage by 0.17%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   85.74%   85.92%   +0.17%     
==========================================
  Files          13       13              
  Lines         828      831       +3     
==========================================
+ Hits          710      714       +4     
+ Misses        118      117       -1     
Impacted Files Coverage Δ
softioc/builder.py 97.00% <75.00%> (+1.12%) ⬆️

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 ea1b85a...146f350. Read the comment docs.

@@ -0,0 +1,5 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't committed any vscode config yet as @Araneidae doesn't use vscode. However as there are now two of us doing this, maybe we should. What do you think @Araneidae ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thinking about it, if we do go down that route we should probably just bring across all the .vscode settings from the template module, rather than what I've done here and just a small selection to get one particular piece working as I wanted.

@Araneidae
Copy link
Collaborator

This is now obsoleted by PR #60

@Araneidae Araneidae closed this Jan 13, 2022
@Araneidae Araneidae deleted the fix_nelm_processing branch January 13, 2022 13:35
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.

Waveform records ignore NELM keyword if initial_value is set
3 participants