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

core: frontend: Add menu items to home #899

Merged
merged 2 commits into from
Mar 23, 2022

Conversation

Williangalvani
Copy link
Member

home

}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you just do:

Suggested change
}
for (const item of this.menus) {
for (const subitem of item.submenus) {
if (!subitem?.advanced || this.settings.is_pirate_mode) {
items.push(subitem)
}
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

not all of them have submenus defined, typescript complains about it without the check. I did something else you may like, tho...

Comment on lines 16 to 17
text: 'General Autopilot settings, allow you to start/stop Ardupilot if using Navigator or SITL, and switch'
+ ' boards if more than one is present',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bit unclear to me, is this a correct understanding?

Suggested change
text: 'General Autopilot settings, allow you to start/stop Ardupilot if using Navigator or SITL, and switch'
+ ' boards if more than one is present',
text: 'General autopilot settings: allow you to start/stop Ardupilot firmware if using Navigator or SITL, and switch'
+ ' boards if more than one is present',

I'm also not certain what "switch boards" means - is the assumption that the others are idle/off if not 'active', or could they all be on and doing stuff, and this just changes which one BlueOS is actively controlling/doing stuff with?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC it is for switching between navigator and pixhawk for people who happen to plug both in simultaneously. we could omit that, lol.

Comment on lines 19 to 24
{
title: 'Firmware',
icon: 'mdi-chip',
route: '/autopilot/firmware',
advanced: false,
text: 'Used to download and flash new firmware to your board',
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to document this as a separate page, or is the combined page happening very soon?

Should possibly specify that it flashes to the "actively selected board", or "current board" or something

Copy link
Member Author

Choose a reason for hiding this comment

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

the change is trivial to do after we merge them, I think this isn't an issue. I renamed to "current flight controller"

Comment on lines 39 to 40
text: 'Manage Mavlink endpoints for internal/external systems. Used this if you need to connect additional'
+ ' mavlink systems to your vehicle',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
text: 'Manage Mavlink endpoints for internal/external systems. Used this if you need to connect additional'
+ ' mavlink systems to your vehicle',
text: 'Manage MAVLink endpoints for internal/external systems. Use this if you need to connect additional'
+ ' MAVLink systems to your vehicle',

icon: 'mdi-map-marker',
route: '/tools/nmea-injector',
advanced: true,
text: 'Used for forwarding UDP NMEA stream into Ardupilot',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
text: 'Used for forwarding UDP NMEA stream into Ardupilot',
text: 'Used for forwarding UDP NMEA streams into Ardupilot',

Or should this be one? ("a UDP NMEA stream")

icon: 'mdi-speedometer',
route: '/tools/network-test',
show: true,
text: 'Test link speed between topside computer and your vehicle',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this for sub, although I'm assuming drone users don't refer to the GCS as a "topside" computer? Not sure what our preference is here (do we want default sub terminology, or actively keep things general to ArduPilot vehicles?)

Copy link
Member Author

Choose a reason for hiding this comment

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

we're currently walking the line... I don't want to be in any sides too much 😅
see my previous comment about "flight controller"

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 can just remove the "topside" reference

text: 'Used for forwarding UDP NMEA stream into Ardupilot',
},
{
title: 'System information',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
title: 'System information',
title: 'System Information',

text: 'Detailed system status information, CPU, memory, disk, and ethernet status',
},
{
title: 'Network test',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
title: 'Network test',
title: 'Network Test',

text: 'View detailed MAVLink traffic coming from your vehicle',
},
{
title: 'Version-chooser',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
title: 'Version-chooser',
title: 'Version Chooser',

icon: 'mdi-cellphone-arrow-down',
route: '/tools/version-chooser',
advanced: false,
text: 'Manage versions and update to latest available',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
text: 'Manage versions and update to latest available',
text: 'Manage BlueOS versions and update to the latest available',

@patrickelectric patrickelectric merged commit db19cb7 into bluerobotics:master Mar 23, 2022
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.

3 participants