-
Notifications
You must be signed in to change notification settings - Fork 65
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
Framebuffer update #60
Changes from 4 commits
6368765
f27b632
0beebdd
0043c78
ffd5e60
a66c507
c72e701
2428d01
16ef979
5f505de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ use tokio_util::io::ReaderStream; | |
use tokio_util::task::TaskTracker; | ||
use futures::{StreamExt, TryStreamExt}; | ||
|
||
use crate::framebuffer; | ||
use crate::qmdl_store::RecordingStore; | ||
use crate::server::ServerState; | ||
|
||
|
@@ -33,6 +34,7 @@ struct AnalysisWriter { | |
writer: BufWriter<File>, | ||
harness: Harness, | ||
bytes_written: usize, | ||
has_warning: bool, | ||
} | ||
|
||
// We write our analysis results to a file immediately to minimize the amount of | ||
|
@@ -47,6 +49,7 @@ impl AnalysisWriter { | |
writer: BufWriter::new(file), | ||
harness: Harness::new_with_all_analyzers(), | ||
bytes_written: 0, | ||
has_warning: false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm i'm not sure why the writer should keep this state around? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes good point, removed. |
||
}; | ||
let metadata = result.harness.get_metadata(); | ||
result.write(&metadata).await?; | ||
|
@@ -59,6 +62,7 @@ impl AnalysisWriter { | |
let row = self.harness.analyze_qmdl_messages(container); | ||
if !row.is_empty() { | ||
self.write(&row).await?; | ||
self.has_warning = ! &row.analysis.is_empty() | ||
} | ||
Ok(self.bytes_written) | ||
} | ||
|
@@ -149,6 +153,8 @@ pub fn run_diag_read_thread( | |
let index = qmdl_store.current_entry.expect("DiagDevice had qmdl_writer, but QmdlStore didn't have current entry???"); | ||
qmdl_store.update_entry_analysis_size(index, analysis_file_len as usize).await | ||
.expect("failed to update analysis file size"); | ||
qmdl_store.update_entry_has_warning(index, analysis_writer.has_warning).await | ||
.expect("failed to update analysis file has warning"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imo we shouldn't be storing whether a warning was detected in the QMDL store, but instead should return from changing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see above! |
||
} | ||
}, | ||
Err(err) => { | ||
|
@@ -172,6 +178,8 @@ pub async fn start_recording(State(state): State<Arc<ServerState>>) -> Result<(S | |
let qmdl_writer = QmdlWriter::new(qmdl_file); | ||
state.diag_device_ctrl_sender.send(DiagDeviceCtrlMessage::StartRecording((qmdl_writer, analysis_file))).await | ||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("couldn't send stop recording message: {}", e)))?; | ||
state.ui_update_sender.send(framebuffer::Color565::Green).await | ||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("couldn't send ui update message: {}", e)))?; | ||
Ok((StatusCode::ACCEPTED, "ok".to_string())) | ||
} | ||
|
||
|
@@ -184,6 +192,8 @@ pub async fn stop_recording(State(state): State<Arc<ServerState>>) -> Result<(St | |
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("couldn't close current qmdl entry: {}", e)))?; | ||
state.diag_device_ctrl_sender.send(DiagDeviceCtrlMessage::StopRecording).await | ||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("couldn't send stop recording message: {}", e)))?; | ||
state.ui_update_sender.send(framebuffer::Color565::White).await | ||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("couldn't send ui update message: {}", e)))?; | ||
Ok((StatusCode::ACCEPTED, "ok".to_string())) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
use std::sync::Arc; | ||
|
||
use crate::qmdl_store::ManifestEntry; | ||
use crate::{framebuffer, qmdl_store::ManifestEntry}; | ||
use crate::server::ServerState; | ||
|
||
use axum::Json; | ||
use axum::extract::State; | ||
use axum::http::StatusCode; | ||
use log::error; | ||
use log::{error, info}; | ||
use serde::Serialize; | ||
use tokio::process::Command; | ||
|
||
|
@@ -121,6 +121,11 @@ pub async fn get_qmdl_manifest(State(state): State<Arc<ServerState>>) -> Result< | |
let qmdl_store = state.qmdl_store_lock.read().await; | ||
let mut entries = qmdl_store.manifest.entries.clone(); | ||
let current_entry = qmdl_store.current_entry.map(|index| entries.remove(index)); | ||
if current_entry.clone().unwrap().has_warning { | ||
info!("a heuristic triggered on this run!"); | ||
state.ui_update_sender.send(framebuffer::Color565::Red).await | ||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("couldn't send ui update message: {}", e)))?; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as per above, this should happen in the diag thread There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved this to analysis_writer.analyze() per your suggestion. |
||
Ok(Json(ManifestStats { | ||
entries, | ||
current_entry, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
use std::borrow::Cow; | ||
|
||
use telcom_parser::lte_rrc::{PCCH_MessageType, PCCH_MessageType_c1, PagingUE_Identity}; | ||
|
||
use super::analyzer::{Analyzer, Event, EventType, Severity}; | ||
use super::information_element::{InformationElement, LteInformationElement}; | ||
|
||
pub struct ExampleAnalyzer{ | ||
pub count: i32, | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imo this kinda thing should be defined in a test file. also this is pedantic i guess but i'd describe it less as an ExampleAnalyzer and more like a DebugAnalyzer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, I renamed it test analyzer and put it behind a gate where it will only run for debug builds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ooh perfect |
||
|
||
impl Analyzer for ExampleAnalyzer{ | ||
fn get_name(&self) -> Cow<str> { | ||
Cow::from("Example Analyzer") | ||
} | ||
|
||
fn get_description(&self) -> Cow<str> { | ||
Cow::from("Always returns true, if you are seeing this you are either a developer or you are about to have problems.") | ||
cooperq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
fn analyze_information_element(&mut self, ie: &InformationElement) -> Option<Event> { | ||
self.count += 1; | ||
if self.count % 100 == 0 { | ||
return Some(Event { | ||
event_type: EventType::Informational , | ||
message: "multiple of 100 events processed".to_string(), | ||
}) | ||
} | ||
let InformationElement::LTE(LteInformationElement::PCCH(pcch_msg)) = ie else { | ||
return None; | ||
}; | ||
let PCCH_MessageType::C1(PCCH_MessageType_c1::Paging(paging)) = &pcch_msg.message else { | ||
return None; | ||
}; | ||
for record in &paging.paging_record_list.as_ref()?.0 { | ||
if let PagingUE_Identity::S_TMSI(_) = record.ue_identity { | ||
return Some(Event { | ||
event_type: EventType::QualitativeWarning { severity: Severity::Low }, | ||
message: "TMSI was provided to cell".to_string(), | ||
}) | ||
} | ||
} | ||
None | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this channel should use descriptive enums of display state, rather than colors directly. something like:
and then the framebuffer code assigns those colors as an implementation detail