update to new api#26
Merged
Merged
Conversation
update to new api re-add old commnets remove emitSafe
There was a problem hiding this comment.
Pull request overview
This PR migrates the chat module from a Qt plugin/interface-based implementation to a “universal” pure C++ implementation that reports results via named events with JSON payloads.
Changes:
- Replaced the Qt
ChatModulePlugin+ChatModuleInterfacedesign with a pure C++ChatModuleImplthat emits JSON strings via anemitEventcallback. - Updated unit tests to exercise
ChatModuleImpland adjusted test include paths for stubs. - Updated module metadata/build inputs for the new universal/codegen setup and removed the old interface header from the build.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_chat_module.cpp |
Updates tests to use ChatModuleImpl and wire emitEvent in test setup. |
tests/CMakeLists.txt |
Adjusts stub include path for the new header include layout. |
src/chat_module_plugin.h |
Replaces Qt plugin interface with ChatModuleImpl API + JSON event contract docs. |
src/chat_module_plugin.cpp |
Implements JSON-emitting callbacks and replaces Qt types/logging with std + nlohmann::json. |
src/chat_module_interface.h |
Removes the legacy Qt plugin interface. |
metadata.json |
Declares universal interface/codegen header and adds Nix package metadata. |
CMakeLists.txt |
Removes the deleted interface header from module sources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <string> | ||
|
|
||
| extern "C" { | ||
| #include "lib/liblogoschat.h" |
| /// Wired automatically by the generated glue layer. | ||
| /// Call this to emit named events to other modules / the host application. | ||
| /// Data is a JSON-encoded string (object or array). | ||
| std::function<void(const std::string& eventName, const std::string& data)> emitEvent; |
Comment on lines
+24
to
30
| ChatModuleImpl::~ChatModuleImpl() | ||
| { | ||
| // Clean up Chat context if it exists | ||
| if (chatCtx) { | ||
| chat_destroy(chatCtx, destroy_callback, this); | ||
| chatCtx = nullptr; | ||
| } | ||
|
|
||
| // Clean up resources | ||
| if (logosAPI) { | ||
| delete logosAPI; | ||
| logosAPI = nullptr; | ||
| } | ||
| } | ||
|
|
||
| void ChatModulePlugin::initLogos(LogosAPI* logosAPIInstance) { | ||
| if (logosAPI) { | ||
| delete logosAPI; | ||
| } | ||
| logosAPI = logosAPIInstance; | ||
| } | ||
|
|
||
| void ChatModulePlugin::emitEvent(const QString& eventName, const QVariantList& data) { | ||
| if (!logosAPI) { | ||
| qWarning() << "ChatModulePlugin: LogosAPI not available, cannot emit" << eventName; | ||
| return; | ||
| } | ||
|
|
||
| LogosAPIClient* client = logosAPI->getClient("chat_module"); | ||
| if (!client) { | ||
| qWarning() << "ChatModulePlugin: Failed to get chat_module client for event" << eventName; | ||
| return; | ||
| } | ||
|
|
||
| client->onEventResponse(this, eventName, data); | ||
| } |
Comment on lines
+5
to
+14
| #include <nlohmann/json.hpp> | ||
|
|
||
| static std::string isoTimestamp() | ||
| { | ||
| auto now = std::chrono::system_clock::now(); | ||
| auto tt = std::chrono::system_clock::to_time_t(now); | ||
| struct tm buf; | ||
| gmtime_r(&tt, &buf); | ||
| char out[32]; | ||
| strftime(out, sizeof(out), "%Y-%m-%dT%H:%M:%SZ", &buf); |
|
|
||
| plugin->emitEvent("chatGetIdResult", eventData); | ||
| nlohmann::json ev; | ||
| ev["clientId"] = message; |
Collaborator
There was a problem hiding this comment.
clientId is the preferred value. Ignore Copilot
jazzz
approved these changes
May 7, 2026
Collaborator
jazzz
left a comment
jazzz
left a comment
There was a problem hiding this comment.
Changes look good. No blocker from my end.
|
|
||
| plugin->emitEvent("chatGetIdResult", eventData); | ||
| nlohmann::json ev; | ||
| ev["clientId"] = message; |
Collaborator
There was a problem hiding this comment.
clientId is the preferred value. Ignore Copilot
Comment on lines
+49
to
+52
| ev["success"] = (callerRet == RET_OK); | ||
| ev["statusCode"] = callerRet; | ||
| ev["message"] = message; | ||
| ev["timestamp"] = isoTimestamp(); |
Collaborator
There was a problem hiding this comment.
[?] Is this schema doc'd anywhere?
Comment on lines
-2
to
-9
| #include <QDebug> | ||
| #include <QCoreApplication> | ||
| #include <QVariantList> | ||
| #include <QDateTime> | ||
| #include <QJsonDocument> | ||
| #include <QJsonObject> | ||
|
|
||
| ChatModulePlugin::ChatModulePlugin() : chatCtx(nullptr) |
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
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.
Update Module to latest version of API
This PR removed QT type dependencies, and migrates to a JSON based approach for EventData