From 1eb4a8d9e9eab8211bc87f0356e611155662d435 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Fri, 14 Feb 2025 10:27:08 -0300 Subject: [PATCH 1/7] Fix reticulate focus --- crates/ark/src/reticulate.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index c3fb09b83..a682dac34 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -55,11 +55,19 @@ impl ReticulateService { fn handle_messages(&self) -> Result<(), anyhow::Error> { loop { - let msg = unwrap!(self.comm.incoming_rx.recv(), Err(err) => { + let msg: CommMsg = unwrap!(self.comm.incoming_rx.recv(), Err(err) => { log::error!("Reticulate: Error while receiving message from frontend: {err:?}"); break; }); + // Eventually, messages will be sent from R to this comm using + // main.get_comm_manager_tx().send() + // We want these to be forwarded to the front-end. + if let CommMsg::Data(_) = msg { + self.comm.outgoing_tx.send(msg)?; + continue; + } + if let CommMsg::Close = msg { break; } From 9f24eeee8cc9d7b4d7a561fef3711944c986f92b Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Fri, 14 Feb 2025 11:30:17 -0300 Subject: [PATCH 2/7] Allow `NULL` as input too --- crates/ark/src/reticulate.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index a682dac34..beb240d79 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -7,6 +7,7 @@ use amalthea::comm::event::CommManagerEvent; use amalthea::socket::comm::CommInitiator; use amalthea::socket::comm::CommSocket; use crossbeam::channel::Sender; +use harp::utils::r_is_null; use harp::RObject; use libr::R_NilValue; use libr::SEXP; @@ -97,11 +98,13 @@ pub unsafe extern "C" fn ps_reticulate_open(input: SEXP) -> Result = input.try_into()?; - - let mut comm_id_guard = RETICULATE_COMM_ID.lock().unwrap(); + let input_code: Option = match r_is_null(input.sexp) { + true => None, + false => Some(input.try_into()?), + }; // If there's an id already registered, we just need to send the focus event + let mut comm_id_guard = RETICULATE_COMM_ID.lock().unwrap(); if let Some(id) = comm_id_guard.deref() { // There's a comm_id registered, we just send the focus event main.get_comm_manager_tx().send(CommManagerEvent::Message( From 4695ccf93c3fef633ca11fcdce966faf2f3e40a5 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 19 Feb 2025 10:40:48 -0300 Subject: [PATCH 3/7] Store a reference to the comm socket instead --- crates/ark/src/reticulate.rs | 53 +++++++++++++++++------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index beb240d79..83ba8c08c 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -19,7 +19,7 @@ use uuid::Uuid; use crate::interface::RMain; -static RETICULATE_COMM_ID: LazyLock>> = LazyLock::new(|| Mutex::new(None)); +static RETICULATE_COMM: LazyLock>> = LazyLock::new(|| Mutex::new(None)); pub struct ReticulateService { comm: CommSocket, @@ -27,7 +27,10 @@ pub struct ReticulateService { } impl ReticulateService { - fn start(comm_id: String, comm_manager_tx: Sender) -> anyhow::Result { + fn start( + comm_id: String, + comm_manager_tx: Sender, + ) -> anyhow::Result { let comm = CommSocket::new( CommInitiator::BackEnd, comm_id.clone(), @@ -45,13 +48,15 @@ impl ReticulateService { .send(event) .or_log_error("Reticulate: Could not open comm."); + let socket = service.comm.clone(); + spawn!(format!("ark-reticulate-{}", comm_id), move || { service .handle_messages() .or_log_error("Reticulate: Error handling messages"); }); - Ok(comm_id) + Ok(socket) } fn handle_messages(&self) -> Result<(), anyhow::Error> { @@ -61,13 +66,7 @@ impl ReticulateService { break; }); - // Eventually, messages will be sent from R to this comm using - // main.get_comm_manager_tx().send() - // We want these to be forwarded to the front-end. - if let CommMsg::Data(_) = msg { - self.comm.outgoing_tx.send(msg)?; - continue; - } + log::trace!("Reticulate: Received message from front end: {msg:?}"); if let CommMsg::Close = msg { break; @@ -81,9 +80,9 @@ impl ReticulateService { .or_log_error("Reticulate: Could not send close message to the front-end"); // Reset the global comm_id before closing - let mut comm_id_guard = RETICULATE_COMM_ID.lock().unwrap(); - log::info!("Reticulate Thread closing {:?}", (*comm_id_guard).clone()); - *comm_id_guard = None; + let mut comm_guard = RETICULATE_COMM.lock().unwrap(); + log::info!("Reticulate Thread closing {:?}", self.comm.comm_id); + *comm_guard = None; Ok(()) } @@ -104,25 +103,23 @@ pub unsafe extern "C" fn ps_reticulate_open(input: SEXP) -> Result Date: Wed, 19 Feb 2025 10:42:25 -0300 Subject: [PATCH 4/7] Add a comment on the reticulate behavior. --- crates/ark/src/reticulate.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index 83ba8c08c..a73e273d2 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -97,9 +97,11 @@ pub unsafe extern "C" fn ps_reticulate_open(input: SEXP) -> Result = match r_is_null(input.sexp) { - true => None, - false => Some(input.try_into()?), + // Reticulate sends `NULL` or a string with the code to be executed in the Python console. + let input_code: Option = if r_is_null(input.sexp) { + None + } else { + Some(input.try_into()?) }; // If there's an id already registered, we just need to send the focus event From 52895fc3eca8d73593d89e16c5efd671907cda81 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 19 Feb 2025 10:49:10 -0300 Subject: [PATCH 5/7] Use a global singleton service --- crates/ark/src/reticulate.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index a73e273d2..4f6fa85ca 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -19,8 +19,10 @@ use uuid::Uuid; use crate::interface::RMain; -static RETICULATE_COMM: LazyLock>> = LazyLock::new(|| Mutex::new(None)); +static RETICULATE_SERVICE: LazyLock>> = + LazyLock::new(|| Mutex::new(None)); +#[derive(Clone)] pub struct ReticulateService { comm: CommSocket, comm_manager_tx: Sender, @@ -30,7 +32,7 @@ impl ReticulateService { fn start( comm_id: String, comm_manager_tx: Sender, - ) -> anyhow::Result { + ) -> anyhow::Result { let comm = CommSocket::new( CommInitiator::BackEnd, comm_id.clone(), @@ -48,15 +50,14 @@ impl ReticulateService { .send(event) .or_log_error("Reticulate: Could not open comm."); - let socket = service.comm.clone(); - + let serv = service.clone(); spawn!(format!("ark-reticulate-{}", comm_id), move || { - service + serv.clone() .handle_messages() .or_log_error("Reticulate: Error handling messages"); }); - Ok(socket) + Ok(service) } fn handle_messages(&self) -> Result<(), anyhow::Error> { @@ -79,13 +80,18 @@ impl ReticulateService { .send(CommMsg::Close) .or_log_error("Reticulate: Could not send close message to the front-end"); - // Reset the global comm_id before closing - let mut comm_guard = RETICULATE_COMM.lock().unwrap(); + // Reset the global service before closing + let mut comm_guard = RETICULATE_SERVICE.lock().unwrap(); log::info!("Reticulate Thread closing {:?}", self.comm.comm_id); *comm_guard = None; Ok(()) } + + fn send_msg_to_frontend(&self, msg: CommMsg) -> Result<(), anyhow::Error> { + self.comm.outgoing_tx.send(msg)?; + Ok(()) + } } // Creates a client instance reticulate can use to communicate with the front-end. @@ -105,10 +111,10 @@ pub unsafe extern "C" fn ps_reticulate_open(input: SEXP) -> Result Result Date: Tue, 25 Feb 2025 13:21:52 -0300 Subject: [PATCH 6/7] Keep a global socket --- crates/ark/src/reticulate.rs | 42 ++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index 4f6fa85ca..0b63e5ec6 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -19,8 +19,7 @@ use uuid::Uuid; use crate::interface::RMain; -static RETICULATE_SERVICE: LazyLock>> = - LazyLock::new(|| Mutex::new(None)); +static RETICULATE_SOCKET: LazyLock>> = LazyLock::new(|| Mutex::new(None)); #[derive(Clone)] pub struct ReticulateService { @@ -29,16 +28,18 @@ pub struct ReticulateService { } impl ReticulateService { - fn start( - comm_id: String, - comm_manager_tx: Sender, - ) -> anyhow::Result { + fn start(comm_id: String, comm_manager_tx: Sender) -> anyhow::Result<()> { let comm = CommSocket::new( CommInitiator::BackEnd, comm_id.clone(), String::from("positron.reticulate"), ); + { + let mut socket_guard = RETICULATE_SOCKET.lock().unwrap(); + *socket_guard = Some(comm.clone()); + } + let service = Self { comm, comm_manager_tx, @@ -50,14 +51,13 @@ impl ReticulateService { .send(event) .or_log_error("Reticulate: Could not open comm."); - let serv = service.clone(); spawn!(format!("ark-reticulate-{}", comm_id), move || { - serv.clone() + service .handle_messages() .or_log_error("Reticulate: Error handling messages"); }); - Ok(service) + Ok(()) } fn handle_messages(&self) -> Result<(), anyhow::Error> { @@ -80,16 +80,13 @@ impl ReticulateService { .send(CommMsg::Close) .or_log_error("Reticulate: Could not send close message to the front-end"); - // Reset the global service before closing - let mut comm_guard = RETICULATE_SERVICE.lock().unwrap(); + // Reset the global soccket before closing log::info!("Reticulate Thread closing {:?}", self.comm.comm_id); - *comm_guard = None; - - Ok(()) - } + { + let mut comm_guard = RETICULATE_SOCKET.lock().unwrap(); + *comm_guard = None; + } - fn send_msg_to_frontend(&self, msg: CommMsg) -> Result<(), anyhow::Error> { - self.comm.outgoing_tx.send(msg)?; Ok(()) } } @@ -111,10 +108,10 @@ pub unsafe extern "C" fn ps_reticulate_open(input: SEXP) -> Result Result Date: Tue, 25 Feb 2025 14:09:50 -0300 Subject: [PATCH 7/7] Keep just the channel used to comunicate with the front-end as a global --- crates/ark/src/reticulate.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/ark/src/reticulate.rs b/crates/ark/src/reticulate.rs index 0b63e5ec6..1e43f980a 100644 --- a/crates/ark/src/reticulate.rs +++ b/crates/ark/src/reticulate.rs @@ -19,7 +19,8 @@ use uuid::Uuid; use crate::interface::RMain; -static RETICULATE_SOCKET: LazyLock>> = LazyLock::new(|| Mutex::new(None)); +static RETICULATE_OUTGOING_TX: LazyLock>>> = + LazyLock::new(|| Mutex::new(None)); #[derive(Clone)] pub struct ReticulateService { @@ -36,8 +37,8 @@ impl ReticulateService { ); { - let mut socket_guard = RETICULATE_SOCKET.lock().unwrap(); - *socket_guard = Some(comm.clone()); + let mut outgoing_tx_guard = RETICULATE_OUTGOING_TX.lock().unwrap(); + *outgoing_tx_guard = Some(comm.outgoing_tx.clone()); } let service = Self { @@ -83,8 +84,8 @@ impl ReticulateService { // Reset the global soccket before closing log::info!("Reticulate Thread closing {:?}", self.comm.comm_id); { - let mut comm_guard = RETICULATE_SOCKET.lock().unwrap(); - *comm_guard = None; + let mut outgoing_tx_guard = RETICULATE_OUTGOING_TX.lock().unwrap(); + *outgoing_tx_guard = None; } Ok(()) @@ -108,10 +109,10 @@ pub unsafe extern "C" fn ps_reticulate_open(input: SEXP) -> Result