Headless runner for console[CPP-318][CPP-313]. #123

john-michaelburke · 2021-09-15T22:20:55Z

Implements

Creates a binary for running the console headless without python or qml frontend.
Moves the shared_state and all subtypes into a "shared_state" module.
Fixes all breaks from moving shared_state module.
Slight change to our logging module to accept a debug parameter which indicates whether or not to print the log messages to the terminal in addition to over capnproto.
Moves the handle_cli function from server.rs -> cli_option.rs
Moves the backend_recv_thread from server.rs -> shared_state.rs (Couldn't think of a more appropriate location for this function as mostly just pushes updates to shared_state).
Adds a makefile command to run headless quicker:

Running in headless mode only command line options work.
Help:
        cargo make headless-run -- --help
Usage:
        cargo make headless-run -- tcp piksi-relay-bb9f2b10e53143f4a816a11884e679cf.ce.swiftnav.com --port=55555

john-michaelburke · 2021-09-15T22:22:06Z

console_backend/src/cli_options.rs

+/// - `connection_state`: The Server state to start a specific connection.
+/// - `client_send`: Client Sender channel for communication from backend to frontend.
+/// - `shared_state`: The shared state for validating another connection is not already running.
+pub fn handle_cli(opt: CliOptions, connection_state: &ConnectionState, shared_state: SharedState) {


This is simply moved out from server.rs, no other changes.

john-michaelburke · 2021-09-15T22:22:47Z

console_backend/src/shared_state.rs

+pub fn backend_recv_thread(
+    connection_state: ConnectionState,
+    client_send: ClientSender,
+    server_recv: channel::Receiver<Vec<u8>>,
+    shared_state: SharedState,
+) {


This is simply moved out from server.rs, no other changes.

Concept wise I'm not keen on moving this here, I think we should disable Python stuff in server.rs, e.g.:

https://github.com/swift-nav/console_pp/pull/116/files#diff-294a0145b9cc648ec69658c67d337bace1dbf930c43f77a4997e050a7791fe0dR38

Or just move it into it's own module server_recv_thread.rs

Done. I like what you did in your PR of moving the py03 stuff to a non default feature. So as to no duplicate some of your work I just set up a server_recv_thread.rs file.

silverjam · 2021-09-15T22:32:54Z

console_backend/src/bin/headless.rs

+        if opt.input.is_none() {
+            eprintln!("\nRunning in headless mode only command line options work.");
+            eprintln!("Help:");
+            eprintln!("\tcargo make headless-run -- --help");


Let's not reference cargo make here since it's part of the build system

console_backend/Cargo.toml

silverjam · 2021-09-15T22:34:32Z

console_backend/src/bin/headless.rs

+            backend_recv_thread(connection_state, client_send, server_recv, shared_state);
+            s.spawn(move |_| {
+                loop {
+                    if client_recv.recv().is_err() {


Can we write these message to the terminal or a log file?

Done. Looks like the files are pretty small too so no need for rotating. I threw in a snap compression wrapper around the writer.

silverjam · 2021-09-15T22:40:44Z

console_backend/src/shared_state.rs

+pub fn backend_recv_thread(
+    connection_state: ConnectionState,
+    client_send: ClientSender,
+    server_recv: channel::Receiver<Vec<u8>>,
+    shared_state: SharedState,
+) {


Concept wise I'm not keen on moving this here, I think we should disable Python stuff in server.rs, e.g.:

https://github.com/swift-nav/console_pp/pull/116/files#diff-294a0145b9cc648ec69658c67d337bace1dbf930c43f77a4997e050a7791fe0dR38

Or just move it into it's own module server_recv_thread.rs

silverjam · 2021-09-15T22:41:52Z

console_backend/src/types.rs

+// use crate::update_tab::UpdateTabUpdate;
+// use crate::utils::{mm_to_m, ms_to_sec, set_connected_frontend};
+
+// use anyhow::{Context, Result as AHResult};


Please don't check in commented out code

Removed. My mistake.

silverjam · 2021-09-15T22:45:21Z

console_backend/src/shared_state.rs

+impl Directory {
+    pub fn new_data_directory() -> Directory {
+        Directory {
+            path: create_data_dir().unwrap(),


Not related to this PR, but we should log a bug/Jira for this since it's ignoring an error for a file system operation (which can fail for many many reasons) so we should handle and report the error instead of .unwraping and potentially panicking.

Set up a ticket:
https://swift-nav.atlassian.net/browse/CPP-320

notoriaga · 2021-09-16T01:49:27Z

console_backend/src/log_panel.rs


        for (idx, item) in slice.iter().enumerate() {
+            if self.debug {
+                println!("{}", item);


eprintln! maybe?

notoriaga · 2021-09-16T02:04:24Z

console_backend/src/bin/headless.rs

+        let opt = CliOptions::from_filtered_cli();
+        let shared_state = SharedState::new();
+        let connection_state = ConnectionState::new(client_send.clone(), shared_state.clone());
+        if opt.input.is_none() {


How about moving the help stuff to the top, something like

fn main() -> Result<()> { let opt = CliOptions::from_filtered_cli(); if opt.input.is_none() { eprintln!("\n\ Running in headless mode only command line options work.\n\ Help:\n\ \tcargo make headless-run -- --help\n\ Usage:\n\ \tcargo make headless-run -- tcp piksi-relay-bb9f2b10e53143f4a816a11884e679cf.ce.swiftnav.com --port=55555" ); return Ok(()); } ... }

to keep the main logic a little cleaner

notoriaga · 2021-09-16T02:07:37Z

console_backend/src/bin/headless.rs

+                    }
+                }
+            })
+            .join().unwrap();


You don't typically have to manually call join() on threads created with scope.spawn(..). the thread is automatically joined before the scope ends (that's why it can borrow things)

Done. I should probably remove the join in the process_messages for the update tab as well.

Co-authored-by: Jason Mobarak <[email protected]>

Headless runner for console.

c3a4993

john-michaelburke commented Sep 15, 2021

View reviewed changes

john-michaelburke marked this pull request as ready for review September 15, 2021 22:24

john-michaelburke requested a review from a team September 15, 2021 22:29

silverjam suggested changes Sep 15, 2021

View reviewed changes

notoriaga approved these changes Sep 16, 2021

View reviewed changes

john-michaelburke and others added 2 commits September 16, 2021 09:59

Update console_backend/Cargo.toml

e57d4c1

Co-authored-by: Jason Mobarak <[email protected]>

Log messages.

fc3f398

john-michaelburke requested a review from silverjam September 17, 2021 01:51

silverjam approved these changes Sep 17, 2021

View reviewed changes

john-michaelburke merged commit e1786a3 into main Sep 17, 2021

john-michaelburke deleted the john-michaelburke/headless branch September 17, 2021 17:33

john-michaelburke changed the title ~~Headless runner for console[CPP-318].~~ Headless runner for console[CPP-318][CPP-313]. Sep 20, 2021

Uh oh!

Headless runner for console[CPP-318][CPP-313]. #123

Headless runner for console[CPP-318][CPP-313]. #123

Uh oh!

Conversation

john-michaelburke commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Implements

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notoriaga Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notoriaga Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

john-michaelburke commented Sep 15, 2021 •

edited

Loading

notoriaga Sep 16, 2021 •

edited

Loading

notoriaga Sep 16, 2021 •

edited

Loading