Add Vehicle Logs

[patrickelectric]

[patrickelectric]

ping @joaoantoniocardoso

[joaoantoniocardoso]

There you are, just a few details and you are good to go

[joaoantoniocardoso]

Do you mind adding an explanation of why changing the receiver buffer size?

maybe:

Suggested change

	const TARGET_RCVBUF: usize = 1024 * 1024; // 1 MB
	let std_sock = socket.into_std()?;
	let sock2 = socket2::Socket::from(std_sock);
	if let Err(e) = sock2.set_recv_buffer_size(TARGET_RCVBUF) {
	warn!("Failed to set SO_RCVBUF to {TARGET_RCVBUF}: {e}");
	}
	let actual = sock2.recv_buffer_size().unwrap_or(0);
	info!("UDP socket SO_RCVBUF set to {actual} bytes (requested {TARGET_RCVBUF})");
	let std_sock: std::net::UdpSocket = sock2.into();
	std_sock.set_nonblocking(true)?;
	let socket = UdpSocket::from_std(std_sock)?;
	let socket = {
	const TARGET_RCVBUF: usize = 1024 * 1024; // 1 MB
	let std_socket = socket.into_std()?;
	let sock2 = socket2::Socket::from(std_socket);
	if let Err(error) = sock2.set_recv_buffer_size(TARGET_RCVBUF) {
	warn!("Failed to set SO_RCVBUF to {TARGET_RCVBUF}: {error}");
	}
	match sock2.recv_buffer_size() {
	Ok(actual) => info!(
	"UDP socket SO_RCVBUF set to {actual} bytes (requested {TARGET_RCVBUF})"
	),
	Err(error) => {
	warn!("Failed to set UDP socket SO_RCVBUF to {TARGET_RCVBUF}: {error:?}")
	}
	}
	let std_socket = sock2.into();
	UdpSocket::from_std(std_socket)?
	};

Note: the socket was already created as nonblocking (from tokio).

[joaoantoniocardoso]

Would you mind explaining the reason in the commit message?

[joaoantoniocardoso]

Following our top-down declaration order convention (as opposed to C), this should be the first thing in the file, in reverse order, followed by LogDownloadStatus, and then the other types. Also, given that DownloadMap is used once, it doesn't add much clarity given the context:

lazy_static! {
    static ref ACTIVE_DOWNLOADS: RwLock<HashMap<String, Arc<LogDownloadSession>>> =
        RwLock::new(HashMap::new());
}

struct LogDownloadSession {
    filename: String,
    bytes_downloaded: AtomicU64,
    total_bytes: u64,
    status: RwLock<LogDownloadStatus>,
    error: RwLock<Option<String>>,
    data: RwLock<Option<Vec<u8>>>,
    cancelled: AtomicBool,
}

#[derive(Debug, Serialize, Clone, PartialEq, Eq)]
#[serde(rename_all = "snake_case")]
pub enum LogDownloadStatus {
    Downloading,
    Complete,
    Error,
}

#[derive(Debug, Clone, Serialize)]
pub struct LogEntry {
    pub id: u16,
    pub time_utc: u32,
    pub size: u32,
}

#[derive(Debug, Serialize, Clone)]
pub struct LogDownloadProgress {
    pub download_id: String,
    pub filename: String,
    pub bytes_downloaded: u64,
    pub total_bytes: u64,
    pub status: LogDownloadStatus,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub error: Option<String>,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum LogClientError {
    TimedOut,
}

[joaoantoniocardoso]

This should be at the top, followed by the others as follows:

pub struct VehicleLogsTab {
    targets: Arc<Mutex<FetchState<Vec<FtpTarget>>>>,
    auto_select_first: Arc<Mutex<bool>>,
    selected_target: Option<FtpTarget>,
    logs: Arc<Mutex<FetchState<Vec<LogEntryUi>>>>,
    status_message: Arc<Mutex<Option<String>>>,
    active_download: Arc<Mutex<Option<ActiveLogDownload>>>,
    erase_confirm: bool,
    needs_log_fetch: bool,
}

#[derive(Debug, Clone, Default)]
struct FtpTarget {
    system_id: u8,
    component_id: u8,
    description: String,
}

#[derive(Debug, Clone)]
struct LogEntryUi {
    id: u16,
    time_utc: u32,
    size: u32,
}

#[derive(Debug, Clone)]
struct ActiveLogDownload {
    download_id: String,
    filename: String,
    bytes_downloaded: u64,
    total_bytes: u64,
    status: String,
    error: Option<String>,
}

#[derive(Debug, Clone)]
enum FetchState<T> {
    Idle,
    Loading,
    Done(T),
    Error(String),
}

[joaoantoniocardoso]

Suggested change

	Err(e) => *t = FetchState::Error(format!("Parse error: {e}")),
	Err(error) => *t = FetchState::Error(format!("Parse error: {error}")),

[joaoantoniocardoso]

Suggested change

	FetchState::Error(ref e) => {
	ui.colored_label(egui::Color32::RED, format!("Error: {e}"));
	FetchState::Error(ref error) => {
	ui.colored_label(egui::Color32::RED, format!("Error: {error}"));

[joaoantoniocardoso]

Suggested change

	FetchState::Error(ref e) => {
	ui.colored_label(egui::Color32::RED, format!("Error: {e}"));
	FetchState::Error(ref error) => {
	ui.colored_label(egui::Color32::RED, format!("Error: {error}"));

[joaoantoniocardoso]

Suggested change

	Err(err) => *l = FetchState::Error(format!("Parse error: {err}")),
	Err(error) => *l = FetchState::Error(format!("Parse error: {error}")),

[joaoantoniocardoso]

Suggested change

	Err(err) => *l = FetchState::Error(err),
	Err(error) => *l = FetchState::Error(error),

[joaoantoniocardoso]

perhaps make this clearer with parentheses?