Refactor re_smart_channel -> re_log_channel #12052
Merged
+740 −1,110
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
emilk added 🚜 refactor 
| Web viewer failed to build.
View image diff on kitdiff. Note: This comment is updated whenever you push a commit. |
jprochazk reviewed Dec 2, 2025
emilk force-pushed the emilk/log-channel branch from 1abaaaf to 196c4ef Compare emilk force-pushed the emilk/log-channel branch from 196c4ef to b4917a1 Compare 
emilk commented Dec 3, 2025
| use crate::{Channel, LogSource, SendError, SmartMessage, SmartMessagePayload}; | ||
| | ||
| #[derive(Clone)] | ||
| pub struct LogSender { |
Member Author
There was a problem hiding this comment.
For some reason git didn't realize this is mostly a rename of the old sender file, but with generics removed
emilk commented Dec 3, 2025
| | ||
| use crate::{Channel, LogSource, SmartMessage, TryRecvError}; | ||
| | ||
| pub struct LogReceiver { |
Member Author
There was a problem hiding this comment.
This is mostly a rename of the old receiver file, but with generics removed
emilk commented Dec 3, 2025
Comment on lines +29 to +34
| /// Call this on each sent message. | ||
| /// | ||
| /// Can be used to wake up the receiver thread. | ||
| pub fn set_waker(&self, waker: impl Fn() + Send + Sync + 'static) { | ||
| *self.channel.waker.write() = Some(Box::new(waker)); | ||
| } |
lucasmerlin approved these changes Dec 3, 2025
Comment on lines 78 to 101
| /// Used to indicate that a sender has left. | ||
| /// | ||
| /// This sends a message down the channel allowing the receiving end to know whether one of the | ||
| /// sender has left, and if so why (if applicable). | ||
| /// | ||
| /// Using a [`LogSender`] after calling `quit` is undefined behavior: the receiving end is free | ||
| /// to silently drop those messages (or worse). | ||
| pub fn quit( | ||
| &self, | ||
| err: Option<Box<dyn std::error::Error + Send>>, | ||
| ) -> Result<(), SendError<Box<SmartMessage>>> { | ||
| self.tx | ||
| .send(SmartMessage { | ||
| source: Arc::clone(&self.source), | ||
| payload: SmartMessagePayload::Quit(err), | ||
| }) | ||
| .map_err(|SendError(msg)| SendError(Box::new(msg)))?; | ||
| | ||
| if let Some(waker) = self.channel.waker.read().as_ref() { | ||
| (waker)(); | ||
| } | ||
| | ||
| Ok(()) | ||
| } |
Member
There was a problem hiding this comment.
Should dropping the Sender also call Quit?
Member Author
There was a problem hiding this comment.
No - the Quit shows intention. I'll add a comment to that effect.
Also the LogSender is clonable, so we really don't wanna quit on drop.
| /// ⚠️ This function must be called from the main thread since some platforms require that | ||
| /// their UI runs on the main thread! ⚠️ | ||
| pub fn show(main_thread_token: crate::MainThreadToken, msgs: Vec<LogMsg>) -> eframe::Result { | ||
| let log_source = re_log_channel::LogSource::Sdk; // Whatever. Nobody is really using this functions anyhow. |
Member
There was a problem hiding this comment.
Then why not get rid of this? 🤔
Member Author
There was a problem hiding this comment.
It's part of our public API. We can mark it #[deprecated] but that feels out-of-bounds for this PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
re_smart_channelis a monster that has evolved over time.This takes a big step towards simplifying it
re_smart_channeltore_log_channelDataSourceMessagenowSender<T>/Receiver<T>toLogSender/LogReceiverSmartMessageSourceandSmartChannelSourceinto a singleLogSource