Addressed reviewer comments.

Signed-off-by: Holger Rapp <HolgerRapp@gmx.net>

everestrs/everestrs/src/lib.rs

@@ -3,7 +3,6 @@ mod schema;
 use argh::FromArgs;
 use serde::de::DeserializeOwned;
 use std::collections::HashMap;
-use std::os::raw::c_char;
 use std::path::PathBuf;
 use std::pin::Pin;
 use thiserror::Error;
@@ -23,10 +22,12 @@ mod ffi {
     struct CommandMeta {
         implementation_id: String,
         name: String,
-        // cxx.rs does not expose a *const c_void type, so we use *const c_char everywhere.
-        // Wherever this shows it means "pointer to some random blob of memory that only Rust
-        // understand what it means and is of no concern to C".
-        obj: *const c_char,
+    }
+
+    extern "Rust" {
+        type Runtime;
+        fn handle_command(self: &Runtime, meta: &CommandMeta, json: JsonBlob) -> JsonBlob;
+        fn on_ready(&self);
     }
 
     struct JsonBlob {
@@ -48,20 +49,11 @@ mod ffi {
 
         /// Registers the callback of the `GenericModule` to be called and calls
         /// `Everest::Module::signal_ready`.
-        unsafe fn signal_ready(
-            self: &Module,
-            // See comment in `CommandMeta::obj`.
-            obj: *const c_char,
-            on_ready: unsafe fn(obj: *const c_char) -> (),
-        );
+        fn signal_ready(self: &Module, rt: &Runtime);
 
         /// Informs the runtime that we implement the command described in `meta` and registers the
         /// `handle_command` method from the `GenericModule` as the handler.
-        unsafe fn provide_command(
-            self: &Module,
-            meta: CommandMeta,
-            on_ready: unsafe fn(&CommandMeta, JsonBlob) -> JsonBlob,
-        );
+        fn provide_command(self: &Module, rt: &Runtime, meta: &CommandMeta);
     }
 }
 
@@ -129,8 +121,16 @@ pub struct Runtime {
 }
 
 impl Runtime {
-    fn ptr_to_module_impl(&self) -> *const c_char {
-        &*self.module_impl.as_ref() as *const dyn GenericModule as *const c_char
+    fn on_ready(&self) {
+        self.module_impl.on_ready();
+    }
+
+    fn handle_command(&self, meta: &ffi::CommandMeta, json: ffi::JsonBlob) -> ffi::JsonBlob {
+        let blob = self
+            .module_impl
+            .handle_command(&meta.implementation_id, &meta.name, json.deserialize())
+            .unwrap();
+        ffi::JsonBlob::from_vec(serde_json::to_vec(&blob).unwrap())
     }
 
     // TODO(hrapp): This function could use some error handling.
@@ -150,47 +150,27 @@ impl Runtime {
 
         // Implement all commands for all of our implementations, dispatch everything to the
         // GenericModule.
-        for (implementation_id, provides) in manifest.provides {
-            let interface_s = module.cpp_module.get_interface(&provides.interface);
+        for (implementation_id, implementation) in manifest.provides {
+            let interface_s = module.cpp_module.get_interface(&implementation.interface);
             let interface: schema::Interface = interface_s.deserialize();
             for (name, _) in interface.cmds {
                 let meta = ffi::CommandMeta {
                     implementation_id: implementation_id.clone(),
                     name,
-                    obj: module.ptr_to_module_impl(),
                 };
-                unsafe {
-                    module.cpp_module.as_ref().unwrap().provide_command(
-                        meta,
-                        |meta: &ffi::CommandMeta, params: ffi::JsonBlob| {
-                            let module_impl = &mut *(meta.obj as *mut T);
-                            let out = match module_impl.handle_command(
-                                &meta.implementation_id,
-                                &meta.name,
-                                params.deserialize(),
-                            ) {
-                                Err(e) => panic!("Error calling command: {e:?}"),
-                                Ok(out) => out,
-                            };
-                            ffi::JsonBlob::from_vec(serde_json::to_vec(&out).unwrap())
-                        },
-                    );
-                }
+
+                module
+                    .cpp_module
+                    .as_ref()
+                    .unwrap()
+                    .provide_command(&module, &meta);
             }
         }
 
         // Since users can choose to overwrite `on_ready`, we can call signal_ready right away.
-        unsafe {
-            module
-                .cpp_module
-                .as_ref()
-                .unwrap()
-                .signal_ready(module.ptr_to_module_impl(), |obj| {
-                    let module_impl = &mut *(obj as *mut T);
-                    module_impl.on_ready();
-                });
-        }
-
+        // TODO(sirver): There were some doubts if this strategy is too inflexible, discuss design
+        // again.
+        module.cpp_module.as_ref().unwrap().signal_ready(&module);
         module
     }
 }

everestrs/everestrs_sys/everestrs_sys.cpp

@@ -59,19 +59,16 @@ JsonBlob Module::initialize() {
   return json2blob(config_->get_manifests().at(module_name));
 }
 
-void Module::signal_ready(const char* obj,
-                          rust::Fn<void(const char*)> on_ready) const {
-  handle_->register_on_ready_handler([on_ready, obj]() { on_ready(obj); });
+void Module::signal_ready(const Runtime& rt) const {
+  handle_->register_on_ready_handler([&rt]() { rt.on_ready(); });
   handle_->signal_ready();
 }
 
-void Module::provide_command(
-    CommandMeta meta,
-    rust::Fn<JsonBlob(const CommandMeta&, JsonBlob)> command_handler) const {
+void Module::provide_command(const Runtime& rt, const CommandMeta& meta) const {
   handle_->provide_cmd(std::string(meta.implementation_id),
                        std::string(meta.name),
-                       [command_handler, meta](json args) {
-                         JsonBlob blob = command_handler(meta, json2blob(args));
+                       [&rt, meta](json args) {
+                         JsonBlob blob = rt.handle_command(meta, json2blob(args));
                          return json::parse(blob.data.begin(), blob.data.end());
                        });
 }

everestrs/everestrs_sys/everestrs_sys.hpp

@@ -9,6 +9,7 @@
 
 struct CommandMeta;
 struct JsonBlob;
+struct Runtime;
 
 class Module {
  public:
@@ -18,11 +19,8 @@ class Module {
   JsonBlob initialize();
   JsonBlob get_interface(rust::Str interface_name) const;
 
-  void signal_ready(const char* obj,
-                    rust::Fn<void(const char*)> on_ready) const;
-  void provide_command(
-      CommandMeta meta,
-      rust::Fn<JsonBlob(const CommandMeta&, JsonBlob)> command_handler) const;
+  void signal_ready(const Runtime&rt) const;
+  void provide_command(const Runtime& rt,const CommandMeta& meta) const;
 
   // TODO(hrapp): Add call_command, publish_variable and subscribe_variable.