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

Derived Telemetry Prototype #7823

Open
scottbell opened this issue Sep 3, 2024 · 10 comments · May be fixed by #7815
Open

Derived Telemetry Prototype #7823

scottbell opened this issue Sep 3, 2024 · 10 comments · May be fixed by #7815
Assignees
Labels
notable_change A change which should be noted in the changelog type:feature Feature. Required intentional design
Milestone

Comments

@scottbell
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Create a new objected in Open MCT that supports a user entered expression (in math.js format) and outputs the result of that expression as telemetry.

@scottbell scottbell linked a pull request Sep 3, 2024 that will close this issue
14 tasks
@charlesh88
Copy link
Contributor

charlesh88 commented Sep 4, 2024

In progress notes while working in the branch. This may totally just be stuff you haven't gotten to yet, but if not, you're welcome:

  • An output of 0 is being rendered as "---".
    • Exp a with test value 0 would expect to output 0.
    • Exp a*b with test values of 0 and 1 respectively would expect to output 0.
    • Exp a+b with -10 and 10 would expect to output 0.
  • When exiting edit mode with "Apply Test Values" enabled, the subsequent browse view works as if test values are being applied - but it should not. Test values should only be applied while in edit mode. Refreshing or navving away and back resets the view with test values apparently disabled. We should immediately update the output field on exiting edit mode to the default value "---" and then allow it to naturally be updated by any live telemetry values if we have them.

charlesh88 added a commit that referenced this issue Sep 5, 2024
- Closes #7823
- Font files updated - do not merge!
- New glyph and data URI background for object.
charlesh88 added a commit that referenced this issue Sep 5, 2024
- Closes #7823
- Tweak to object description.
charlesh88 added a commit that referenced this issue Sep 5, 2024
- Closes #7823
- Added new `ObjectPathString.vue` UI component, based on `ObjectPath.vue`.
charlesh88 added a commit that referenced this issue Sep 5, 2024
- Closes #7823
- Significant mods for style and CSS classing. Still WIP, more coming.
charlesh88 added a commit that referenced this issue Sep 5, 2024
- Closes #7823
- Code cleanup and tweaks.
@charlesh88
Copy link
Contributor

charlesh88 commented Sep 5, 2024

Changes so far:

  • New component /ui/components/ObjectPathString.vue used instead of ObjectPath.
  • New CSS defs in comps.scss.
  • Layout, styling cleanups.
  • Test values section of telemetry ref elements now hidden when not in edit mode.
  • Tweaks to messaging.
  • Removed error icon applied to main holder.
  • Refined resize behavior of expression entry textarea.

Screenshot 2024-09-04 at 6 33 24 PM
Screenshot 2024-09-04 at 6 33 46 PM

@charlesh88
Copy link
Contributor

charlesh88 commented Sep 5, 2024

Todos

@charlesh88

  • Continue style refinements, particularly code validation / error message area.
  • Refine main 'output' field at top of UI to better handle long values, like arrays.

@scottbell
Copy link
Contributor Author

Great work @charlesh88! I think I addressed the issues you described, and am working toward some e2e tests now. One question though: do we want to make the expression itself more prominent? It's rather small now, and is probably the main thing I want to be looking at when I bring up the view?

charlesh88 added a commit that referenced this issue Sep 6, 2024
- Closes #7823
- Updated Derived Telemetry glyph design.
- Set `CompsViewProvider.js` to use the correct icon.
charlesh88 added a commit that referenced this issue Sep 6, 2024
- Closes #7823
- Final sanding and polishing.
- New discrete item style `c-output-featured` added to controls.scss
  - Applied to 'Current Output' section of both Derived Telemetry and Condition Sets.
- TODOs:
  - [ ] Check for low-risk regressions in Condition Sets browse and edit modes.
  - [ ] New tests for ObjectPathString.vue component.
@charlesh88
Copy link
Contributor

charlesh88 commented Sep 6, 2024

Agree on the smallness of the expression; gave it a monospace font which looks bigger and is just better. Have pushed what I think are near complete changes for derived telemetry:

  • Final sanding and polishing.
  • New discrete item style c-output-featured added to controls.scss.
    • Applied to 'Current Output' section of both Derived Telemetry and Condition Sets.
  • Added icons and styling to expression validation message. Message now shows in browse mode if invalid.
  • Monospace font used in expression input and browse mode display.

TODOs:

  • Check for low-risk regressions in Condition Sets browse and edit modes.
  • New tests for ObjectPathString.vue component.

@scottbell
Copy link
Contributor Author

scottbell commented Sep 6, 2024

Testing Notes

  1. Create a new Derived Telemetry object
  2. Drop a couple of SWGs on it
  3. Change the parameter names from a to x, and b to y
  4. Add a simple math.js expression: x*y - ensure expression is valid
  5. Change expression to a*b - ensure expression is marked invalid
  6. Change back to a*b, and add some test values to the parameters (e.g., 2 and 5), then toggle "Apply Test Data". Ensure the output matches the multiplication.
  7. Go to the inspector tab in edit mode, and change the output format to %0.2f and save. Ensure floating point is formatted to only two decimals places in the output.
  8. Untoggle "Apply Test Data" and put time conductor in real time mode. Ensure "Current Output" has a changing number that looks reasonable.
  9. Drop Derived telemetry into an overlay plot along with the SWGs. Ensure both historical and realtime data plots.

@scottbell scottbell linked a pull request Sep 6, 2024 that will close this issue
14 tasks
@unlikelyzero unlikelyzero added notable_change A change which should be noted in the changelog type:feature Feature. Required intentional design and removed type:enhancement labels Sep 10, 2024
@unlikelyzero unlikelyzero added this to the Target:4.1.0 milestone Sep 10, 2024
@unlikelyzero
Copy link
Collaborator

Needs developer review but the tests and a11y look great. I made some comments.

Also we need to create good release notes and be sure to call this experimental. I went ahead and moved this to the 4.1.0 release

@charlesh88 charlesh88 changed the title Derived Telemtry Prototype Derived Telemetry Prototype Sep 17, 2024
charlesh88 added a commit that referenced this issue Nov 14, 2024
- Markup/CSS sanding and shimming for derived telemetry front-end.
- Toggle switch CSS improved to use `gap` instead of left margin.
@charlesh88
Copy link
Contributor

charlesh88 commented Nov 14, 2024

Reference Images

Illustrates expected wrapping / alignment behavior at different widths.
Screenshot 2024-11-14 at 10 08 18 AM
Screenshot 2024-11-14 at 10 07 47 AM
Screenshot 2024-11-14 at 10 07 14 AM
Screenshot 2024-11-14 at 10 07 07 AM

@charlesh88
Copy link
Contributor

charlesh88 commented Nov 14, 2024

I've finished front-end sanding and shimming. @scottbell there were some minor markup changes with some v-ifs moving around a bit and some consolidation of containers. I put it through the wringer myself, but could you have a look and make sure I didn't break something?

@akhenry Otherwise, this is ready for review. Because changes are in the combined-rodap-stuff branch, I'm not going to create a PR yet given there might be more outstanding 'stuff'.

scottbell pushed a commit that referenced this issue Nov 15, 2024
- Markup/CSS sanding and shimming for derived telemetry front-end.
- Toggle switch CSS improved to use `gap` instead of left margin.
@scottbell
Copy link
Contributor Author

@charlesh88 I did a cursory look (made a derived telemetry, testing good & bad equations, etc.) and it seemed to work well. FYI, I did cherry-pick your new commit over to the telemetry-comps branch that has the PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notable_change A change which should be noted in the changelog type:feature Feature. Required intentional design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants