Skip to content

RDK-60795 : Added Cryptography plugin. (#6305)#6458

Merged
srikanth-vv merged 1 commit intordkcentral:release/8.1_p45ukfrom
sborushevsky:RDK-60795_8.1_p45uk
Feb 18, 2026
Merged

RDK-60795 : Added Cryptography plugin. (#6305)#6458
srikanth-vv merged 1 commit intordkcentral:release/8.1_p45ukfrom
sborushevsky:RDK-60795_8.1_p45uk

Conversation

@sborushevsky
Copy link
Contributor

  • RDK-57366 : Added Cryptography plugin.

  • Added VaultId(). Code cleanup.

  • Renamed plugin.

  • RDK-57366 : Used Svalbard plugin sources from R4_4 branch.

  • Added changelog.

* RDK-57366 : Added Cryptography plugin.

* Added VaultId(). Code cleanup.

* Renamed plugin.

* RDK-57366 : Used Svalbard plugin sources from R4_4 branch.

* Added changelog.
Copilot AI review requested due to automatic review settings February 17, 2026 17:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Cryptography service plugin module (implemented as CryptographyExtAccess plus an RPC-exposed CryptographyImplementation) and wires it into the build/config system, along with initial documentation and changelog.

Changes:

  • Introduces the Cryptography plugin sources (CryptographyExtAccess + CryptographyImplementation RPC service).
  • Adds build/config integration for the new plugin (CMake + .config/.conf.in templates).
  • Adds initial README.md and CHANGELOG.md for the service.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
CMakeLists.txt Adds conditional add_subdirectory(Cryptography) to integrate the new plugin into the top-level build.
Cryptography/CMakeLists.txt Defines the CryptographyExtAccess shared library build, dependencies, install, and config generation.
Cryptography/CryptographyExtAccess.h Declares the CryptographyExtAccess plugin and remote-connection notification sink.
Cryptography/CryptographyExtAccess.cpp Implements plugin lifecycle (Initialize/Deinitialize) and connects to the RPC service.
Cryptography/CryptographyRPC.cpp Adds CryptographyImplementation service exposing ICryptography via RPC.
Cryptography/Module.h Declares module name and plugin headers for the new module.
Cryptography/Module.cpp Adds module declaration for build reference export.
Cryptography/CryptographyExtAccess.config CMake config mapping for plugin root object config values.
Cryptography/CryptographyExtAccess.conf.in Template for generated plugin config (callsign + root object settings).
Cryptography/README.md Adds minimal build instructions.
Cryptography/CHANGELOG.md Adds initial changelog content and guidance text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +62 to +63
printf("CryptographyExtAccess - Connection Id - %u\n",_connectionId);
_implementation->Configure(_service);
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

_implementation->Configure(_service) return value is ignored, and the plugin still reports successful initialization even if configuration fails. Other plugins (e.g. Packager) treat a non-Core::ERROR_NONE configure result as initialization failure and unregister/clean up. Please check the return code and, on failure, release/unregister and return a non-empty error string so the framework can fail the plugin startup cleanly.

Suggested change
printf("CryptographyExtAccess - Connection Id - %u\n",_connectionId);
_implementation->Configure(_service);
printf("CryptographyExtAccess - Connection Id - %u\n", _connectionId);
const uint32_t configureResult = _implementation->Configure(_service);
if (configureResult != Core::ERROR_NONE) {
message = _T("CryptographyExtAccess configuration failed.");
// Clean up partially initialized state so the framework can fail startup cleanly.
Deinitialize(_service);
}

Copilot uses AI. Check for mistakes.
Comment on lines +109 to +135
uint32_t Configure(PluginHost::IShell* framework) override
{
uint32_t result(Core::ERROR_NONE);
Config config;

config.FromString(framework->ConfigLine());

_cryptography = Exchange::ICryptography::Instance("");

Core::ProxyType<RPC::InvokeServer> server = Core::ProxyType<RPC::InvokeServer>::Create(&Core::IWorkerPool::Instance());

_rpcLink = new ExternalAccess(Core::NodeId(config.Connector.Value().c_str()), this, _T(""), server);

if (_rpcLink != nullptr) {
if (_rpcLink->IsListening() == false) {
delete _rpcLink;
_rpcLink = nullptr;

if (_cryptography != nullptr) {
_cryptography->Release();
_cryptography = nullptr;
}
}
}

return result;
}
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Configure() always returns Core::ERROR_NONE even if _cryptography could not be created (Exchange::ICryptography::Instance returning nullptr) or if the RPC communicator fails to listen (the code deletes _rpcLink and releases _cryptography, but still reports success). Please return an appropriate error code (and ideally add a TRACE) when initialization fails so callers can detect misconfiguration instead of running with no exported interfaces.

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +45
find_package(${NAMESPACE}Definitions REQUIRED)

Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Cryptography/CMakeLists.txt calls find_package(${NAMESPACE}Definitions REQUIRED) twice (once at line 40 and again at line 44). This is redundant and makes the build file noisier to maintain; please drop the duplicate call.

Suggested change
find_package(${NAMESPACE}Definitions REQUIRED)

Copilot uses AI. Check for mistakes.

SERVICE_REGISTRATION(CryptographyImplementation, 1, 0);
}
} /* namespace WPEFramework::Plugin */
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The namespace closing comment is misleading: the final } closes namespace WPEFramework, but the comment says namespace WPEFramework::Plugin. Please adjust the comment to match the actual scope to avoid confusion when navigating the file.

Suggested change
} /* namespace WPEFramework::Plugin */
} /* namespace WPEFramework */

Copilot uses AI. Check for mistakes.
add_subdirectory(PerformanceMetrics)
endif()

if(PLUGIN_CRYPTOGRAPHY)
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The top-level build guard uses PLUGIN_CRYPTOGRAPHY, but this repository’s plugin enable flags are defined via services.cmake and there is currently no PLUGIN_CRYPTOGRAPHY option/definition. As a result this subdirectory will never be built unless the variable is manually injected. Please add the corresponding option/definition (or switch this guard to the actual plugin flag used by this module, e.g. PLUGIN_CRYPTOGRAPHYEXTACCESS, to match the CMake/config naming).

Suggested change
if(PLUGIN_CRYPTOGRAPHY)
if(PLUGIN_CRYPTOGRAPHYEXTACCESS)

Copilot uses AI. Check for mistakes.
@srikanth-vv srikanth-vv merged commit af0f534 into rdkcentral:release/8.1_p45uk Feb 18, 2026
12 of 13 checks passed
@sborushevsky sborushevsky deleted the RDK-60795_8.1_p45uk branch February 18, 2026 17:05
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

Comments