Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ProcedureObjectName and ProcedureObjectValue to OsConfig Schema #856

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

wupeka
Copy link
Contributor

@wupeka wupeka commented Feb 4, 2025

Description

Add ProcedureObjectName and ProcedureObjectValue fields to OsConfig schema and OsConfigResource
Add support for multiple Components in OsConfigResource.

Checklist

  • I have read the contribution guidelines.
  • I have merged the latest dev branch prior to this PR submission.
  • I submitted this PR against the dev branch.

@wupeka wupeka requested a review from a team as a code owner February 4, 2025 08:03
@wupeka wupeka force-pushed the wpk-add-procedure-to-osconfig branch from 4b190ab to f632d24 Compare February 4, 2025 12:11
@wupeka
Copy link
Contributor Author

wupeka commented Feb 4, 2025

@microsoft-github-policy-service agree company="Microsoft"

@wupeka wupeka force-pushed the wpk-add-procedure-to-osconfig branch 2 times, most recently from 17970d9 to 8beb725 Compare February 4, 2025 12:24
src/adapters/mc/Common.h Outdated Show resolved Hide resolved
Copy link
Contributor

@MariusNi MariusNi left a comment

Choose a reason for hiding this comment

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

🕐

@wupeka wupeka force-pushed the wpk-add-procedure-to-osconfig branch from 8beb725 to 9237261 Compare February 4, 2025 17:33
payloadSize = (int)strlen(serializedValue);
if (NULL != (payloadString = malloc(payloadSize + 1)))
{
const OsConfigComponent *component = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

const OsConfigComponent *component

Pure C style requires all variables locally declared withing a statement of a function to be defined at the beginning/top of the statement. To respect this old style and for consistency with existing code, can you please move the definition on line 421 at top, between lines 389 and 396? Please repeat the same for all other locals that would fall into this case.

payloadSize = (int)strlen(serializedValue);
if (NULL != (payloadString = malloc(payloadSize + 1)))
{
const OsConfigComponent *component = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

OsConfigComponent

Where is OsConfigComponent defined?

LogInfo(context, GetLog(), "[%s] MmiSet(%s, %s, '%.*s', %d) returned %d",
who, g_componentName, objectName, payloadSize, payloadString, payloadSize, mpiResult);

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not do this. This is the role of the Modules Manager. Adapters cannot call MMI directly. Adapters can only call the MPI.

Please remove this entire function and keep the NRP existing functions for get and set that talk to OSConfig over the MPI REST API.

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