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

BeingRecordingMsg -> SetRecordingInfo #2149

Merged
merged 2 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions crates/re_data_store/src/log_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use nohash_hasher::IntMap;

use re_arrow_store::{DataStoreConfig, TimeInt};
use re_log_types::{
component_types::InstanceKey, ArrowMsg, BeginRecordingMsg, Component as _, ComponentPath,
DataCell, DataRow, DataTable, EntityPath, EntityPathHash, EntityPathOpMsg, LogMsg, PathOp,
RecordingId, RecordingInfo, RowId, TimePoint, Timeline,
component_types::InstanceKey, ArrowMsg, Component as _, ComponentPath, DataCell, DataRow,
DataTable, EntityPath, EntityPathHash, EntityPathOpMsg, LogMsg, PathOp, RecordingId,
RecordingInfo, RowId, SetRecordingInfo, TimePoint, Timeline,
};

use crate::{Error, TimesPerTimeline};
Expand Down Expand Up @@ -169,8 +169,8 @@ pub struct LogDb {
/// Set by whomever created this [`LogDb`].
pub data_source: Option<re_smart_channel::Source>,

/// Comes in a special message, [`LogMsg::BeginRecordingMsg`].
recording_msg: Option<BeginRecordingMsg>,
/// Comes in a special message, [`LogMsg::SetRecordingInfo`].
recording_msg: Option<SetRecordingInfo>,

/// Where we store the entities.
pub entity_db: EntityDb,
Expand All @@ -187,7 +187,7 @@ impl LogDb {
}
}

pub fn recording_msg(&self) -> Option<&BeginRecordingMsg> {
pub fn recording_msg(&self) -> Option<&SetRecordingInfo> {
self.recording_msg.as_ref()
}

Expand Down Expand Up @@ -224,7 +224,7 @@ impl LogDb {
crate::profile_function!();

match &msg {
LogMsg::BeginRecordingMsg(msg) => self.add_begin_recording_msg(msg),
LogMsg::SetRecordingInfo(msg) => self.add_begin_recording_msg(msg),
LogMsg::EntityPathOpMsg(_, msg) => {
let EntityPathOpMsg {
row_id,
Expand All @@ -241,7 +241,7 @@ impl LogDb {
Ok(())
}

fn add_begin_recording_msg(&mut self, msg: &BeginRecordingMsg) {
fn add_begin_recording_msg(&mut self, msg: &SetRecordingInfo) {
self.recording_msg = Some(msg.clone());
}

Expand Down
12 changes: 5 additions & 7 deletions crates/re_data_ui/src/log_msg.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use re_log_types::{
ArrowMsg, BeginRecordingMsg, DataTable, EntityPathOpMsg, LogMsg, RecordingInfo,
};
use re_log_types::{ArrowMsg, DataTable, EntityPathOpMsg, LogMsg, RecordingInfo, SetRecordingInfo};
use re_viewer_context::{UiVerbosity, ViewerContext};

use super::DataUi;
Expand All @@ -15,7 +13,7 @@ impl DataUi for LogMsg {
query: &re_arrow_store::LatestAtQuery,
) {
match self {
LogMsg::BeginRecordingMsg(msg) => msg.data_ui(ctx, ui, verbosity, query),
LogMsg::SetRecordingInfo(msg) => msg.data_ui(ctx, ui, verbosity, query),
LogMsg::EntityPathOpMsg(_, msg) => msg.data_ui(ctx, ui, verbosity, query),
LogMsg::ArrowMsg(_, msg) => msg.data_ui(ctx, ui, verbosity, query),
LogMsg::Goodbye(_, _) => {
Expand All @@ -25,16 +23,16 @@ impl DataUi for LogMsg {
}
}

impl DataUi for BeginRecordingMsg {
impl DataUi for SetRecordingInfo {
fn data_ui(
&self,
_ctx: &mut ViewerContext<'_>,
ui: &mut egui::Ui,
_verbosity: UiVerbosity,
_query: &re_arrow_store::LatestAtQuery,
) {
ui.code("BeginRecordingMsg");
let BeginRecordingMsg { row_id: _, info } = self;
ui.code("SetRecordingInfo");
let SetRecordingInfo { row_id: _, info } = self;
let RecordingInfo {
application_id,
recording_id,
Expand Down
6 changes: 3 additions & 3 deletions crates/re_log_encoding/src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,11 @@ impl<R: std::io::Read> Iterator for Decoder<R> {
#[test]
fn test_encode_decode() {
use re_log_types::{
ApplicationId, BeginRecordingMsg, LogMsg, RecordingId, RecordingInfo, RecordingSource,
RecordingType, RowId, Time,
ApplicationId, LogMsg, RecordingId, RecordingInfo, RecordingSource, RecordingType, RowId,
SetRecordingInfo, Time,
};

let messages = vec![LogMsg::BeginRecordingMsg(BeginRecordingMsg {
let messages = vec![LogMsg::SetRecordingInfo(SetRecordingInfo {
row_id: RowId::random(),
info: RecordingInfo {
application_id: ApplicationId("test".to_owned()),
Expand Down
8 changes: 4 additions & 4 deletions crates/re_log_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ pub enum LogMsg {
/// A new recording has begun.
///
/// Should usually be the first message sent.
BeginRecordingMsg(BeginRecordingMsg),
SetRecordingInfo(SetRecordingInfo),

/// Server-backed operation on an [`EntityPath`].
EntityPathOpMsg(RecordingId, EntityPathOpMsg),
Expand All @@ -222,7 +222,7 @@ pub enum LogMsg {
impl LogMsg {
pub fn recording_id(&self) -> &RecordingId {
match self {
Self::BeginRecordingMsg(msg) => &msg.info.recording_id,
Self::SetRecordingInfo(msg) => &msg.info.recording_id,
Self::EntityPathOpMsg(recording_id, _) | Self::ArrowMsg(recording_id, _) => {
recording_id
}
Expand All @@ -231,14 +231,14 @@ impl LogMsg {
}
}

impl_into_enum!(BeginRecordingMsg, LogMsg, BeginRecordingMsg);
impl_into_enum!(SetRecordingInfo, LogMsg, SetRecordingInfo);

// ----------------------------------------------------------------------------

#[must_use]
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub struct BeginRecordingMsg {
pub struct SetRecordingInfo {
pub row_id: RowId,
pub info: RecordingInfo,
}
Expand Down
27 changes: 13 additions & 14 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,14 @@ impl RecordingStreamInner {
) -> RecordingStreamResult<Self> {
let batcher = DataTableBatcher::new(batcher_config)?;

// TODO(cmc): BeginRecordingMsg is a misnomer; it's idempotent.
{
re_log::debug!(
app_id = %info.application_id,
rec_id = %info.recording_id,
"setting recording info",
);
sink.send(
re_log_types::BeginRecordingMsg {
re_log_types::SetRecordingInfo {
row_id: re_log_types::RowId::random(),
info: info.clone(),
}
Expand Down Expand Up @@ -425,7 +424,7 @@ impl RecordingStream {
/// You can create a [`RecordingInfo`] with [`crate::new_recording_info`];
///
/// The [`RecordingInfo`] is immediately sent to the sink in the form of a
/// [`re_log_types::BeginRecordingMsg`].
/// [`re_log_types::SetRecordingInfo`].
///
/// You can find sinks in [`crate::sink`].
///
Expand Down Expand Up @@ -486,7 +485,7 @@ fn forwarding_thread(
"setting recording info",
);
new_sink.send(
re_log_types::BeginRecordingMsg {
re_log_types::SetRecordingInfo {
row_id: re_log_types::RowId::random(),
info: info.clone(),
}
Expand Down Expand Up @@ -981,22 +980,22 @@ mod tests {
// First message should be a set_recording_info resulting from the original sink swap to
// buffered mode.
match msgs.pop().unwrap() {
LogMsg::BeginRecordingMsg(msg) => {
LogMsg::SetRecordingInfo(msg) => {
assert!(msg.row_id != RowId::ZERO);
similar_asserts::assert_eq!(rec_info, msg.info);
}
_ => panic!("expected BeginRecordingMsg"),
_ => panic!("expected SetRecordingInfo"),
}

// Second message should be a set_recording_info resulting from the later sink swap from
// buffered mode into in-memory mode.
// This arrives _before_ the data itself since we're using manual flushing.
match msgs.pop().unwrap() {
LogMsg::BeginRecordingMsg(msg) => {
LogMsg::SetRecordingInfo(msg) => {
assert!(msg.row_id != RowId::ZERO);
similar_asserts::assert_eq!(rec_info, msg.info);
}
_ => panic!("expected BeginRecordingMsg"),
_ => panic!("expected SetRecordingInfo"),
}

// Third message is the batched table itself, which was sent as a result of the implicit
Expand Down Expand Up @@ -1046,22 +1045,22 @@ mod tests {
// First message should be a set_recording_info resulting from the original sink swap to
// buffered mode.
match msgs.pop().unwrap() {
LogMsg::BeginRecordingMsg(msg) => {
LogMsg::SetRecordingInfo(msg) => {
assert!(msg.row_id != RowId::ZERO);
similar_asserts::assert_eq!(rec_info, msg.info);
}
_ => panic!("expected BeginRecordingMsg"),
_ => panic!("expected SetRecordingInfo"),
}

// Second message should be a set_recording_info resulting from the later sink swap from
// buffered mode into in-memory mode.
// This arrives _before_ the data itself since we're using manual flushing.
match msgs.pop().unwrap() {
LogMsg::BeginRecordingMsg(msg) => {
LogMsg::SetRecordingInfo(msg) => {
assert!(msg.row_id != RowId::ZERO);
similar_asserts::assert_eq!(rec_info, msg.info);
}
_ => panic!("expected BeginRecordingMsg"),
_ => panic!("expected SetRecordingInfo"),
}

let mut rows = {
Expand Down Expand Up @@ -1126,11 +1125,11 @@ mod tests {
// First message should be a set_recording_info resulting from the original sink swap
// to in-memory mode.
match msgs.pop().unwrap() {
LogMsg::BeginRecordingMsg(msg) => {
LogMsg::SetRecordingInfo(msg) => {
assert!(msg.row_id != RowId::ZERO);
similar_asserts::assert_eq!(rec_info, msg.info);
}
_ => panic!("expected BeginRecordingMsg"),
_ => panic!("expected SetRecordingInfo"),
}

// The underlying batcher is never flushing: there's nothing else.
Expand Down
6 changes: 3 additions & 3 deletions crates/re_sdk_comms/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@ impl CongestionManager {
#[allow(clippy::match_same_arms)]
match msg {
// we don't want to drop any of these
LogMsg::BeginRecordingMsg(_)
| LogMsg::EntityPathOpMsg(_, _)
| LogMsg::Goodbye(_, _) => true,
LogMsg::SetRecordingInfo(_) | LogMsg::EntityPathOpMsg(_, _) | LogMsg::Goodbye(_, _) => {
true
}

LogMsg::ArrowMsg(_, arrow_msg) => self.should_send_time_point(&arrow_msg.timepoint_max),
}
Expand Down
4 changes: 2 additions & 2 deletions crates/re_viewer/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ impl App {
while let Ok(msg) = self.rx.try_recv() {
let recording_id = msg.recording_id();

let is_new_recording = if let LogMsg::BeginRecordingMsg(msg) = &msg {
let is_new_recording = if let LogMsg::SetRecordingInfo(msg) = &msg {
re_log::debug!("Opening a new recording: {:?}", msg.info);
self.state.selected_rec_id = Some(recording_id.clone());
true
Expand Down Expand Up @@ -1884,7 +1884,7 @@ fn save_database_to_file(

let begin_rec_msg = log_db
.recording_msg()
.map(|msg| LogMsg::BeginRecordingMsg(msg.clone()));
.map(|msg| LogMsg::SetRecordingInfo(msg.clone()));

let ent_op_msgs = log_db
.iter_entity_op_msgs()
Expand Down