Skip to content

PSG - Bug fixes for interfaces such as IBrowser.h#240

Open
nxtum wants to merge 21 commits intomasterfrom
PSG/December2025Fixes
Open

PSG - Bug fixes for interfaces such as IBrowser.h#240
nxtum wants to merge 21 commits intomasterfrom
PSG/December2025Fixes

Conversation

@nxtum
Copy link
Copy Markdown
Contributor

@nxtum nxtum commented Feb 26, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 26, 2026 12:54
@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/ThunderTools/240/rdkcentral/ThunderTools

  • Commit: 587658f

Report detail: gist'

Copy link
Copy Markdown
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

This PR updates the Plugin Skeleton Generator to better handle real-world Thunder interface headers (e.g., optional EXTERNAL, nested types/aliases used by notification signatures), improves notification code generation, and adds support for choosing an output directory.

Changes:

  • Extend interface parsing to capture using aliases and nested type declarations for more robust parameter type qualification in generated notification code.
  • Add generation support for notification helper implementations in source, selective JSON “J*” includes, and optional OOP configuration scaffolding.
  • Allow the generator to emit the plugin into a user-selected output directory and update scripts/templates accordingly.

Reviewed changes

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

Show a summary per file
File Description
PluginSkeletonGenerator/utils/FileUtils.py Adds a post-processing pass to normalize generated output around Metadata<...> token spacing.
PluginSkeletonGenerator/templates/.plugin_source.txt Adds {{SOURCE_NOTIFY_IMPL}} injection point in generated source.
PluginSkeletonGenerator/templates/.plugin_implementation.txt Adds placeholders for copied using aliases and optional OOP configure method emission.
PluginSkeletonGenerator/templates/.plugin_header.txt Makes the “private members” access specifier driven by a placeholder.
PluginSkeletonGenerator/parser/Parser.py Improves interface detection (EXTERNAL optional) and captures using aliases + nested type names.
PluginSkeletonGenerator/menu/Menu.py Adds output directory prompt, stricter yes/no handling, and plugin-name validation.
PluginSkeletonGenerator/generators/PluginRepositoryGenerator.py Writes generated plugin under output_dir/plugin_name instead of always CWD.
PluginSkeletonGenerator/data/FileData.py Adds type-qualification helpers for notification params, deduped includes, event-only JSON includes, and in-process notification implementations.
PluginSkeletonGenerator/core/PluginBlueprint.py Tracks @event notification entries separately and adds output_dir to blueprint.
PluginSkeletonGenerator/core/GeneratorCoordinator.py Threads output_dir through to the blueprint.
PluginSkeletonGenerator/CombinedPlugins.sh Updates scripted stdin answers to match the new prompts (output dir added).

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

@mhughesacn
Copy link
Copy Markdown

Hi @nxtum : All code files are supposed to have proper Apache headers please.

@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/ThunderTools/240/rdkcentral/ThunderTools

  • Commit: 807fd39

Report detail: gist'

Copilot AI review requested due to automatic review settings February 26, 2026 13:40
@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/ThunderTools/240/rdkcentral/ThunderTools

  • Commit: 3a2e837

Report detail: gist'

@nxtum
Copy link
Copy Markdown
Contributor Author

nxtum commented Feb 26, 2026

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

* Protex Server Path: /home/blackduck/github/ThunderTools/240/rdkcentral/ThunderTools

* Commit: [3a2e837](https://github.com/rdkcentral/ThunderTools/commit/3a2e837b145a32b2124ab696d18fd24573c3ebdc)

Report detail: gist'

it doesnt like the word token

Copy link
Copy Markdown
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

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.


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

@github-actions
Copy link
Copy Markdown

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22444624696/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 26, 2026 14:18
@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/ThunderTools/240/rdkcentral/ThunderTools

  • Commit: 77997fc

Report detail: gist'

Copy link
Copy Markdown
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

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.


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

@github-actions
Copy link
Copy Markdown

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22446053936/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

@rdkcmf-jenkins
Copy link
Copy Markdown
Contributor

b'## WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

  • Upvote reason: Thank you

  • Commit: 77997fc
    '

@github-actions
Copy link
Copy Markdown

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22483695122/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

Copilot AI review requested due to automatic review settings March 2, 2026 14:23
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 2, 2026

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22580185447/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

Copy link
Copy Markdown
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

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.


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

Copy link
Copy Markdown
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

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.


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

You can also share your feedback on Copilot code review. Take the survey.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22671327857/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22671341293/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

Copilot AI review requested due to automatic review settings March 4, 2026 14:06
@github-actions

This comment was marked as outdated.

This comment was marked as outdated.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 4, 2026 14:20
@github-actions

This comment was marked as outdated.

This comment was marked as outdated.

@nxtum nxtum requested a review from VeithMetro March 4, 2026 14:33
@github-actions
Copy link
Copy Markdown

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-22892842439/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

Copy link
Copy Markdown
Contributor

@VeithMetro VeithMetro left a comment

Choose a reason for hiding this comment

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

Good changes and many, many bug fixes, great job! I just found a few minor things in the generated plugins:

  1. Preconditions/Terminations/Controls collapsed into Preconditions only
    Across all *Preconditions skeleton variants (e.g. InProcessConfigPreconditions), the generator now puts every subsystem into Preconditions and leaves Terminations/Controls empty
  2. Unnecessary const_cast in Revoked handlers?
    In multiple files (e.g. OutOfProcess.cpp), the Revoked handler applies const_cast to a pointer already returned as non-const by QueryInterface. Is this necessary as we cannot always tell whether that is const or not? Can't we detect that somehow?
  3. Spacing before commas in subsystem lists
    Minor formatting issue: { subsystem::GRAPHICS , subsystem::NOT_GRAPHICS , subsystem::TIME } has spaces before commas

@nxtum
Copy link
Copy Markdown
Contributor Author

nxtum commented Mar 16, 2026

2. **Unnecessary const_cast in Revoked handlers?**
   In multiple files (e.g. OutOfProcess.cpp), the Revoked handler applies const_cast to a pointer already returned as non-const by QueryInterface. Is this necessary as we cannot always tell whether that is const or not? Can't we detect that somehow?

@VeithMetro

            template <typename REQUESTEDINTERFACE>
            REQUESTEDINTERFACE* QueryInterface()
            {
                void* baseInterface(QueryInterface(REQUESTEDINTERFACE::ID));

                if (baseInterface != nullptr) {
                    return (reinterpret_cast<REQUESTEDINTERFACE*>(baseInterface));
                }

                return (nullptr);
            }

            template <typename REQUESTEDINTERFACE>
            const REQUESTEDINTERFACE* QueryInterface() const
            {
                const void* baseInterface(const_cast<IUnknown*>(this)->QueryInterface(REQUESTEDINTERFACE::ID));

                if (baseInterface != nullptr) {
                    return (reinterpret_cast<const REQUESTEDINTERFACE*>(baseInterface));
                }

                return (nullptr);
            }

As remote is a const Core::IUnknown*
auto* revokedInterface = remote->QueryInterfaceExchange::IBrowser::INotification();
will call:

const REQUESTEDINTERFACE* QueryInterface() const

and since then the unregister method wants an interface that is not const, the constcast is needed here

Copilot AI review requested due to automatic review settings March 30, 2026 11:23
@github-actions
Copy link
Copy Markdown

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-23742287475/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

Copy link
Copy Markdown
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

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.


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

@github-actions
Copy link
Copy Markdown

Plugin skeleton preview (latest run):
https://rdkcentral.github.io/ThunderTools/PluginSkeletonGenerator/previews/pr-240/run-23742486391/

AI summary of generated diff:
AI summary unavailable (HTTP 403). Check workflow logs.

@VeithMetro VeithMetro self-requested a review March 30, 2026 13:02
Copy link
Copy Markdown
Contributor

@VeithMetro VeithMetro left a comment

Choose a reason for hiding this comment

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

One last thing, it does not compile now, because we changed to C++17 lately, and we use it in Thunder now 😄 You have to make sure that the CMake files that are generated don't have C++11 hardcoded as they do now

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.

5 participants