Skip to content

feat: use logos-rust-sdk#25

Open
osmaczko wants to merge 1 commit into
masterfrom
rust-bindings-exploration
Open

feat: use logos-rust-sdk#25
osmaczko wants to merge 1 commit into
masterfrom
rust-bindings-exploration

Conversation

@osmaczko
Copy link
Copy Markdown
Contributor

@osmaczko osmaczko commented May 7, 2026

Re-implement chat_module in Rust as a c-ffi module wrapping libchat via
logos-rust-sdk, replacing the C++ Qt plugin around liblogoschat.

  • metadata.json: switch interface from universal to c-ffi; depend on
    delivery_module at runtime instead of bundling a chat library.
  • Generate include/chat_module.h from src/lib.rs with cbindgen at build.
  • Resolve Cargo git deps reproducibly via flake outputHashes (no
    vendored source tree).
  • Drop C++ sources, gtest harness, and liblogoschat stubs.

@osmaczko osmaczko marked this pull request as draft May 7, 2026 14:01
@osmaczko osmaczko force-pushed the rust-bindings-exploration branch from c439579 to 84874ea Compare May 13, 2026 13:29
@osmaczko osmaczko marked this pull request as ready for review May 13, 2026 13:33
Comment thread src/events.rs
Comment on lines +3 to +10
//! Logos modules normally push notifications to subscribers by emitting
//! plugin events through the SDK — the same mechanism we consume on the
//! receive side via `LogosModuleSDK::on(...)` (e.g. delivery_module's
//! `messageReceived`). The c-ffi module track has no story yet for
//! module-emitted events (logos-cpp-generator has no event-declaration
//! convention, the generated Qt glue has no emit forwarding, and
//! logos-rust-sdk has no Rust-side emit API), so we buffer events here
//! and consumers drain them via `chat_module_drain_events_json()`.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iurimatias would it be possible to expose plugin->emitEvent to logos-rust-sdk?

Comment thread src/delivery.rs
// boundaries. inbound.rs decodes on the receive side.
//
// TODO: drop once delivery_module exposes binary send / messageReceived
// (QByteArray) and logos-rust-sdk / logos-cpp-sdk add a bytes Param type.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iurimatias just raising awareness, as of now binary blobs must be translated to a string-safe wrapper to fit the json envelope. Possible solution would be to replace json params with a binary format that has a native byte-string type (like cbor):

 char* logos_sdk_call_method_sync(
     const char* plugin_name,
     const char* method_name,
-    const char* params_json
+    const uint8_t* params_cbor,
+    size_t params_len
 );

wdyt?

@osmaczko osmaczko force-pushed the rust-bindings-exploration branch from 84874ea to 736c266 Compare May 15, 2026 09:20
Re-implement chat_module in Rust as a c-ffi module wrapping libchat via
logos-rust-sdk, replacing the C++ Qt plugin around liblogoschat.

- metadata.json: switch interface from universal to c-ffi; depend on
  delivery_module at runtime instead of bundling a chat library.
- Generate include/chat_module.h from src/lib.rs with cbindgen at build.
- Resolve Cargo git deps reproducibly via flake outputHashes (no
  vendored source tree).
- Drop C++ sources, gtest harness, and liblogoschat stubs.
@osmaczko osmaczko force-pushed the rust-bindings-exploration branch from 736c266 to 9992d59 Compare May 15, 2026 09:33
Copy link
Copy Markdown
Collaborator

@igor-sirotin igor-sirotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noice!
(I didn't quite check the Rust code, only high-level)

Comment thread Cargo.toml
@@ -0,0 +1,37 @@
[package]
name = "chat_module"
version = "1.0.0"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version = "1.0.0"
version = "0.1.0"

Comment thread metadata.json
"codegen": {
"impl_header": "chat_module_plugin.h"
"name": "chat_module",
"version": "1.0.0",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"version": "1.0.0",
"version": "0.1.0",

Comment thread metadata.json
"name": "chat_module",
"version": "1.0.0",
"description": "Chat module for Logos",
"author": "Logos Core Team",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"author": "Logos Core Team",
"author": "Logos Team",

Comment thread .gitignore
@@ -1,14 +1,5 @@
.DS_Store
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep .DS_Store, it's classic MacOS thing

Comment thread CMakeLists.txt
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new CMake file looks quite complex, would be nice to find ways to simplify it in the future.

Comment thread src/lib.rs
///
/// - `instance_path` – writable directory owned by this module instance
/// - `delivery_preset` – e.g. "logos.dev" (defaults to "logos.dev" if NULL)
/// - `tcp_port` – TCP port for the embedded Waku node (e.g. 60000)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TCP port shouldn't be exposed to client. Instead, a random port should be chosen automatically by logos-delivery-module. This is not yet the case, but we will will make it work — logos-co/logos-delivery-module#18 (comment).

At the moment the suggested workaround is to set portsShift

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants