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

Fix scalar input forms #141

Merged
merged 4 commits into from
Oct 16, 2023
Merged

Conversation

SMorettini
Copy link
Contributor

@SMorettini SMorettini commented Sep 11, 2023

Originating Project/Creator
Affected Component fprime_gds/flask

Change Description

Original change: Added the modifier number to the scalar argument. This will cast the value to a number and not return a string.((https://v2.vuejs.org/v2/guide/forms.html#number)) --> This change was breaking the usage of hex, bin and octal because there is not such representation in Javascript.

The new change consists of updating the squash function so any number is actually converted to a number.

Rationale

This fix is required to have commands containing argument arrays of numbers. Without the Fix, Vue will send to the GDS server an array like '["1", "2",...]'. The parsing of the array will fail in Python with the error "Failed to validate all arguments".

After the fix, the array will be sent to the GDS backend as '[1, 2,...]' and the parsing of the array will work fine.

@thomas-bc
Copy link
Collaborator

Glancing over this, am I understanding correctly that this resolves nasa/fprime#2273 ? Or is that a separate case?

@SMorettini
Copy link
Contributor Author

I didn't see that issue but yes, this pull request resolved exactly that problem.

@Tetragramm
Copy link

This PR resolves an issue I'm seeing with multi-element parameters. Not a case noted in either this PR or the linked issue nasa/fprime#2273.
Ex:

struct MultiParams {
  param_a: F32,
  param_b: F32
}

@SMorettini
Copy link
Contributor Author

For testing my change you could: clone my branch https://github.com/SMorettini/fprime-gds/tree/Fix-scalar-input-forms in your laptop, and then install the customize gds with: pip install path/to/my/local/gds. Then if you run fprime-gds you should have the one with the fix.

@SMorettini SMorettini marked this pull request as draft September 19, 2023 12:28
@SMorettini
Copy link
Contributor Author

SMorettini commented Sep 19, 2023

The previous solution was working but created a new bug, I implemented a new solution that solve the original problem without impacting unsigned integer input.

@SMorettini SMorettini marked this pull request as ready for review September 19, 2023 13:13
@LeStarch
Copy link
Collaborator

@SMorettini I went ahead and edited this to use full type names to ensure that the code does not inadvertently read types starting with letters. Care to review my one commit?

@thomas-bc care to review?

LeStarch
LeStarch previously approved these changes Oct 12, 2023
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

Awesome change! I touched it up to ensure a previous bug we've seen has not resurfaced.

Copy link
Contributor Author

@SMorettini SMorettini left a comment

Choose a reason for hiding this comment

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

Thank you for the change, I didn't have time to make it in the last weeks.
I cannot approve it because I created the pull request, anyway, I agree with your changes.

@LeStarch
Copy link
Collaborator

Since I did not see booleans addressed in these if trees, which caused errors, I also fixed: nasa/fprime#2320

@LeStarch LeStarch merged commit 8c0d105 into nasa:devel Oct 16, 2023
12 checks passed
@SMorettini SMorettini deleted the Fix-scalar-input-forms branch October 17, 2023 07:54
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.

4 participants