Parameter handling fixes#4
Open
the-programmer wants to merge 2 commits into
Open
Conversation
added 2 commits
December 19, 2022 11:05
Made CmdAddParameter more strict
JustusRijke
requested changes
Dec 22, 2022
Contributor
JustusRijke
left a comment
JustusRijke
left a comment
There was a problem hiding this comment.
Hi Tim,
Great work, thanks!
Udt_Parameter string lengths were shortened to make sure CSV lines are less than 256 characters. The pull requests reverts this change. If the parameter has some long field strings (for example with nasty floats like -1.7976931348623158e307 and long descriptions) the CSV file will be corrupt.
Could you take a look at that? Simple solution is to change UDT_Parameter again, more futureproof would be to allow longer CSV record strings.
Next step (on our side) would be to combine the 2 parameter file handler implementations with overlapping functionality (FB_ParameterFileHandler and FB_ParameterSelector) into 1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Made a new way to load the parameters.
This unit "trusts" the data that is stored PERSISTENT part based on bLoadedFromPersistent.
If this is set to TRUE, the code uses the value stored in the PERSISTENT memory.
If it is set to FALSE, the value from the CSV is used (if found).
This behavior can be changed via ForceCsv.txt and ForceFactory.txt.
Second, I made CmdAddParameter more strict.
Initially it only checked the number. Now it checks the number and the name in order to prevent indexes from being re-used.