Skip to content

mctpd: add dbus method RegisterVDMTypeSupport#132

Merged
jk-ozlabs merged 1 commit into
CodeConstruct:mainfrom
msnidhin:vdm_msg_type_support
Dec 4, 2025
Merged

mctpd: add dbus method RegisterVDMTypeSupport#132
jk-ozlabs merged 1 commit into
CodeConstruct:mainfrom
msnidhin:vdm_msg_type_support

Conversation

@msnidhin

Copy link
Copy Markdown
Contributor

Add a new DBus method RegisterVDMTypeSupport to support vendor defined message support as responder
Add a new control command handler for get vdm message

Tested:
Unit tests passed
Tested by sending GetVDM command from another endpoint

@msnidhin
msnidhin force-pushed the vdm_msg_type_support branch 2 times, most recently from 0d4a51d to f65eed4 Compare November 28, 2025 17:33

@jk-ozlabs jk-ozlabs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks pretty good, thanks!

I think your decision for using a variant in the registration makes sense, we should just ensure that we get an appropriate dbus error returned when an incorrect variant type is used.

A few comments inline, but they're all fairly minor. The test cases may warrant splitting up though.

Comment thread src/mctpd.c
Comment thread src/mctpd.c Outdated
Comment thread src/mctpd.c
Comment thread tests/test_mctpd.py Outdated
Comment thread src/mctpd.c Outdated
@msnidhin
msnidhin force-pushed the vdm_msg_type_support branch 2 times, most recently from 345b4a9 to 0648bd1 Compare December 1, 2025 19:06
@msnidhin
msnidhin requested a review from jk-ozlabs December 1, 2025 19:09
@jk-ozlabs

Copy link
Copy Markdown
Member

New commit looks good, but can you squash that single fix into the main change?

@msnidhin
msnidhin force-pushed the vdm_msg_type_support branch from 99b77f1 to 317225a Compare December 4, 2025 02:44
@msnidhin

msnidhin commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

New commit looks good, but can you squash that single fix into the main change?

Sure. Rebased the branch

@jk-ozlabs
jk-ozlabs merged commit 317225a into CodeConstruct:main Dec 4, 2025
3 checks passed
@jk-ozlabs

Copy link
Copy Markdown
Member

Merged, thanks!

Add a new DBus method RegisterVDMTypeSupport to support vendor defined
message support as responder
Add a new control command handler for get vdm message

Tested:
Unit tests passed
Tested by sending GetVDM command from another endpoint

Signed-off-by: Nidhin MS <nidhin.ms@intel.com>
Signed-off-by: PavanKumarIntel <pavanx.kumar.martha@intel.com>
@jk-ozlabs

Copy link
Copy Markdown
Member

I guess one follow-up question though: we should probably report 7e / 7f in the Supported Types when one VDM type has been registered, right?

@jk-ozlabs

Copy link
Copy Markdown
Member

(the spec doesn't seem to provide any specific guidance here)

@msnidhin

msnidhin commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

I guess one follow-up question though: we should probably report 7e / 7f in the Supported Types when one VDM type has been registered, right?

Agree.. that sounds good.
I’m calling both RegisterTypeSupport and RegisterVDMSupport now.

Should I open another PR?

@jk-ozlabs

Copy link
Copy Markdown
Member

Yep, best as an separate PR for that.

@msnidhin

msnidhin commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

Hi @jk-ozlabs
Which python formatter are you using to format test scripts ?

@jk-ozlabs

Copy link
Copy Markdown
Member

We don't have one in place at the moment, but I'm mainly going for 80-column lines, to match the rest of the codebase.

@msnidhin
msnidhin deleted the vdm_msg_type_support branch December 7, 2025 08:01
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.

3 participants