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

Document and fix up query parameters and valid query parameter values / ranges. #101

Closed
MatthewHink opened this issue Mar 23, 2018 · 3 comments
Assignees
Labels

Comments

@MatthewHink
Copy link
Contributor

MatthewHink commented Mar 23, 2018

Description

It's not obvious what query parameters are valid on every route.
You have to read the code to find out.
They get changed around and deprecated without comment or testing.
There are no examples showing what might be expected.

Impetus is this change: https://github.com/vapor-ware/synse-server/pull/96/files#diff-17f57b682366acd0bad5e60d91375623R115

Above should change to rpm to set_speed_rpm and percent to set_speed_percent. Sooner or later we could introduce another query parameter in units of rpm or another in units of percent. The set_speed_ prefix at least gives a clue that we are setting current speed.

Examples:
reset_speed_rpm (The speed to run the fan after controller reset)
watchdog_speed_rpm (The speed to run the fan after when the watchdog kicks in)
Likewise with percent. (reset_speed_percent, watchdog_speed_percent)

Related Issues

Close Criteria

Notes

@edaniszewski
Copy link
Contributor

between #109 and #105, would you consider this particular issue fixed, or is there additional work that should happen

@MatthewHink MatthewHink self-assigned this Mar 25, 2018
@MatthewHink
Copy link
Contributor Author

I need to look at this. Self assigned.

@edaniszewski
Copy link
Contributor

Closing this. Since this was opened, a bunch of work was done to validate query params, error when invalid query params were given, error when query params are given but unsupported by the route, and the API docs were updated to document all current query params.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants