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

feat: Support core-keeper for registry and config. #520

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eaton-coreymutter
Copy link
Contributor

@eaton-coreymutter eaton-coreymutter commented Nov 8, 2024

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/device-sdk-c/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?) (no others yet)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?) (tested a similar changeset locally, have not yet spun up a local stock-EdgeX test setup)
  • I have opened a PR for the related docs change (if not, why?) N/A

Testing Instructions

New Dependency Instructions (If applicable)

{
free(keeper->topic_root);
}
if (impl)
Copy link
Member

Choose a reason for hiding this comment

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

This check is redundant because keeper is cast from impl and has been checked at the beginning.

} keeper_impl_t;

static int32_t edgex_keeper_client_notify(void *impl, const iot_data_t *request, const iot_data_t *pathparams, const iot_data_t *params, iot_data_t **reply);
#define CONF_PREFIX "edgex/v3/"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define CONF_PREFIX "edgex/v3/"

unused macro

Comment on lines +231 to +238
err_msg = "'key' or 'value' member not found in object";
}
}
else
{
err_msg = "An element of 'response' is not a JSON object";
}
} /* End loop over responses */
Copy link
Member

Choose a reason for hiding this comment

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

break the loop when an error occurs


// iot_log_info(logger, "At key-root alloc, service->name %p", keeper->service->name);
keeper->key_root = calloc(URL_BUF_SIZE, 1);
snprintf(keeper->key_root, URL_BUF_SIZE-1, "edgex/v3/%s", keeper->service->name);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
snprintf(keeper->key_root, URL_BUF_SIZE-1, "edgex/v3/%s", keeper->service->name);
snprintf(keeper->key_root, URL_BUF_SIZE-1, "edgex/v4/%s", keeper->service->name);

{
t1 = iot_time_msecs ();
*err = EDGEX_OK;
ccReady = edgex_keeper_get_tree(impl, "edgex/v3/core-common-config-bootstrapper", err);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ccReady = edgex_keeper_get_tree(impl, "edgex/v3/core-common-config-bootstrapper", err);
ccReady = edgex_keeper_get_tree(impl, "edgex/v4/core-common-config-bootstrapper", err);

devsdk_nvpairs_free (ccReady);
}

result = edgex_keeper_get_tree(impl, "edgex/v3/core-common-config-bootstrapper/all-services", err);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
result = edgex_keeper_get_tree(impl, "edgex/v3/core-common-config-bootstrapper/all-services", err);
result = edgex_keeper_get_tree(impl, "edgex/v4/core-common-config-bootstrapper/all-services", err);

result = originalResult;

devsdk_nvpairs *privateConfig = NULL;
privateConfig = edgex_keeper_get_tree(impl, "edgex/v3/core-common-config-bootstrapper/device-services", err);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
privateConfig = edgex_keeper_get_tree(impl, "edgex/v3/core-common-config-bootstrapper/device-services", err);
privateConfig = edgex_keeper_get_tree(impl, "edgex/v4/core-common-config-bootstrapper/device-services", err);

{
tree[strlen(tree) - 1] = '\0';
}
strcat(tree, "/#");
Copy link
Member

Choose a reason for hiding this comment

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

MQTT wildcards are not functional in the current sdk.

device-sdk-c/src/c/bus.c

Lines 45 to 54 in b4a60c1

static edgex_handler_fn edgex_bus_match_handler (edgex_bus_t *bus, const char *path, iot_data_t *params, void **ctx)
{
edgex_handler_fn h = NULL;
iot_data_list_iter_t iter;
pthread_mutex_lock (&bus->mtx);
iot_data_list_iter (bus->handlers, &iter);
while (iot_data_list_iter_next (&iter))
{
const edgex_bus_endpoint_t *ep = iot_data_address (iot_data_list_iter_value (&iter));
if (strncmp (path, ep->base, strlen (ep->base)) == 0)
However, this only affects the "listen for config changes" feature and does not break the core functionality of the device service. A separate github issue (#523) has been opened to address this problem.

}
strcat(tree, "/#");
iot_log_info(keeper->lc, "Subscribing to Keeper notifications on message bus at %s", tree);
edgex_bus_register_handler(keeper->service->msgbus, tree, impl, edgex_keeper_client_notify);
Copy link
Member

Choose a reason for hiding this comment

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

This can only subscribe to edgex/configs/edgex/v4/{service_name}/# topic. It also requires subscribing to edgex/configs/edgex/v4/core-common-config-bootstrapper/all-services/# and edgex/configs/edgex/v4/core-common-config-bootstrapper/device-services/#

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