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

Astro #943

Merged
merged 22 commits into from
Dec 10, 2023
Merged

Astro #943

merged 22 commits into from
Dec 10, 2023

Conversation

jmthomas
Copy link
Member

No description provided.

@jmthomas jmthomas requested a review from ryanmelt November 20, 2023 04:51
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (6bf1244) 75.68% compared to head (59a6abc) 75.62%.
Report is 2 commits behind head on main.

Files Patch % Lines
...ool-tlmgrapher/src/tools/TlmGrapher/TlmGrapher.vue 38.46% 8 Missing ⚠️
...smonitor/src/tools/LimitsMonitor/LimitsControl.vue 66.66% 3 Missing and 1 partial ⚠️
...cketviewer/src/tools/PacketViewer/PacketViewer.vue 50.00% 3 Missing ⚠️
...-tlmviewer/src/tools/TlmViewer/NewScreenDialog.vue 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #943      +/-   ##
==========================================
- Coverage   75.68%   75.62%   -0.06%     
==========================================
  Files         620      620              
  Lines       43900    43946      +46     
  Branches      890      902      +12     
==========================================
+ Hits        33224    33234      +10     
- Misses      10576    10613      +37     
+ Partials      100       99       -1     
Flag Coverage Δ
frontend 60.39% <72.13%> (-0.17%) ⬇️
python 84.08% <ø> (-0.06%) ⬇️
ruby-api 48.62% <ø> (ø)
ruby-backend 80.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ryanmelt ryanmelt left a comment

Choose a reason for hiding this comment

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

That's a lot of updates!

@jmthomas
Copy link
Member Author

That's a lot of updates!

I'd like to go over this in detail when you get back

# Build docs tool
COPY --from=docs . packages/openc3-cosmos-tool-docs/
RUN cd packages/openc3-cosmos-tool-docs/scripts && ruby generate_docs_from_yaml.rb PLUGIN && yarn
RUN ["/openc3/plugins/docker-package-build.sh", "openc3-cosmos-tool-docs"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Docs seems to take the longest so I moved things around

@@ -7,27 +7,29 @@ VERTICAL
HORIZONTAL
VERTICALBOX "Position and Velocity"
LABELVALUE <%= target_name %> ADCS POSX WITH_UNITS 20 CENTER
# We recommend using these built-in colors from 1-8
SETTING TEXTCOLOR var(--color-data-visualization-1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Example of using the Astro visualization colors ... this is what we recommend but they can still use color names or RGB hex values.

@@ -48,7 +48,7 @@ export default {
},
data() {
return {
title: 'COSMOS Autonomic',
title: 'Autonomic',
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed 'COSMOS' from all tool titles to shorten them up

single-line
hide-details
style="max-width: 350px"
Copy link
Member Author

Choose a reason for hiding this comment

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

All search boxes now prepend-inner-icon for search, are clearable, outlined, dense, and have a max-width

@@ -73,6 +80,7 @@ export default {
search: '',
headers: [
{ text: 'Time', value: 'time_nsec', width: 250 },
{ text: 'Level', value: 'level', sortable: false },
Copy link
Member Author

Choose a reason for hiding this comment

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

Added level with Astro icon to the limits events table

} else {
this.bottomHeight = 0
}
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Height can still be changed but this fixes the issue where you forget to set the height and it's just 0 and doesn't appear. Basically I'm setting a sensible default.

label="Notifications"
:status="iconStatus"
:notifications="unreadNotifications.length"
></rux-monitoring-icon>
Copy link
Member Author

Choose a reason for hiding this comment

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

Use Astro monitoring-icon for the app bar

@@ -273,6 +269,8 @@ export default {
created: function () {
this.showToast = localStorage.notoast === 'false'
this.subscribe()
// TODO How does this get updated after initialization
this.alerts = this.$store.state.notifyHistory
Copy link
Member Author

Choose a reason for hiding this comment

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

Alerts / notifications still needs some work. I think all the information is here. I left the current implementation where the bell icon always shows and everything is basically a notification.

]
.map((element) => self.capitalize(element))
.sort()
.join(', ')
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just showing all the roles. Astro icon will just truncate the list if it gets too large but our default roles work really well like "Admin, Viewer" or "Operator, Viewer".

Ruby files must be required to be available to call in other code.
Files are first required from the target's lib folder. If no file is found the
Ruby system path is checked which includes the base openc3/lib folder.
description: Ruby files optionally be required to explicitly declare dependencies.
Copy link
Member Author

Choose a reason for hiding this comment

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

It's optional. I'd be ok removing it but I guess it's interesting to explicitly declare dependencies.

Copy link
Member

@ryanmelt ryanmelt left a comment

Choose a reason for hiding this comment

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

Just need to fix cfs doc

@@ -96,7 +96,7 @@ Note we're connecting to the COSMOS network (`docker network ls`) and exposing t

```bash
docker build -t cfs .
docker run --cap-add CAP_SYS_RESOURCE --net=openc3-cosmos-network --name cfs -p1234:1234 -p1235:1235 cfs
docker run --cap-add CAP_SYS_RESOURCE --net=openc3-cosmos-network --name cfs -p1234:1234/udp -p1235:1235/udp cfs
Copy link
Member

Choose a reason for hiding this comment

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

Other commenter said not to add /udp to 1235

Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to fix this.

Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
3.9% 3.9% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@jmthomas jmthomas merged commit bd8c0b2 into main Dec 10, 2023
23 of 25 checks passed
@jmthomas jmthomas deleted the astro branch December 10, 2023 19:51
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.

2 participants