Enabled greengrass components to receive direct message by opting for RECEIVE_ONLY mode - #1164
Enabled greengrass components to receive direct message by opting for RECEIVE_ONLY mode#1164tushar-aws wants to merge 1 commit into
Conversation
| @@ -143,6 +143,10 @@ static GgError rpc_subscribe(void *ctx, GgMap params, uint32_t handle) { | |||
|
|
|||
| bool virtual = false; | |||
There was a problem hiding this comment.
be careful using virtual here in cpp virtual itself is a keyword, but since this is C, it should be fine?
There was a problem hiding this comment.
switched it to virtual <> is_virtual.
| size_t count, | ||
| uint32_t handle, | ||
| uint8_t qos, | ||
| bool virtual |
There was a problem hiding this comment.
virtual in header file is more dangerous. nit: switch to is_virtual
| } | ||
|
|
||
| if (!matched) { | ||
| GG_LOGW( |
There was a problem hiding this comment.
@archigup would this be an expensive operation for gg lite? do we need to demote this or its fine to keep as is. For a scenario where the core device receives lots of unsolicited messages
There was a problem hiding this comment.
I'd say this is fine. A device should not receive anything it did not subscribe to. If it is, then something is wrong - either we should be subscribed, or the cloud incorrectly thinks that we are.
| } | ||
|
|
||
| if (!matched) { | ||
| GG_LOGW( |
There was a problem hiding this comment.
I'd say this is fine. A device should not receive anything it did not subscribe to. If it is, then something is wrong - either we should be subscribed, or the cloud incorrectly thinks that we are.
| if ((i == index) || (topic_filter_len[i] == 0) || sub_virtual[i]) { | ||
| continue; |
There was a problem hiding this comment.
Should we check for handle[index] == handle[i] here too?
Same handle subscribing to same topic multiple times would be left out and will not be unsubscribed from the cloud in the above check. Or am i overthinking this scenario? :)
There was a problem hiding this comment.
No, this scenario will not exists because two duplicate slots are never both alive at the same moment when the check runs. The loop looks at them one at a time, and erases each before moving on.
AniruddhaKanhere
left a comment
AniruddhaKanhere
left a comment
There was a problem hiding this comment.
This looks good to me. Thank you @tushar-aws!
282f05e to
8229f26
Compare
… RECEIVE_ONLY subscriptionMode.
8229f26 to
ec4ab07
Compare
5 participants
Description
Adds an on-device path for AWS IoT Core direct messages. A component can
subscribe over IPC with
subscriptionMode=RECEIVE_ONLY; iotcored registers thetopic filter for local routing only and never issues a cloud SUBSCRIBE. Any
message that reaches the device on a matching filter — including direct messages
addressed to the device by client id, which arrive over the existing connection
with no subscription — is delivered to the subscribing component. A message
matching no subscriber is now dropped and logged instead of silently discarded.
Changes by component:
ggipcd (
SubscribeToIoTCore): new optionalsubscriptionMode.RECEIVE_ONLYmaps to a routing-only (virtual) subscription; absent, null,and
SUBSCRIBE_AND_RECEIVEkeep the existing cloud-subscribe path unchanged;any other value is rejected with
InvalidArgumentsError(fail closed ratherthan silently creating a paid cloud subscription).
qosis ignored forRECEIVE_ONLYsince there is no cloud subscription to configure.iotcored + core bus (
aws_iot_mqttsubscribe): newvirtualflag.A virtual registration sends no cloud SUBSCRIBE, sends no UNSUBSCRIBE on close,
is not re-subscribed on MQTT reconnect, and does not keep a cloud subscription
alive for a filter it shares with a cloud subscription. Inbound dispatch
matches virtual and cloud slots identically (by topic filter), which is what
lets a virtual slot receive direct messages. A message matching no slot is
dropped with a warning log.
Type of Change
Checklist
nix flake check -Llocallyaws-greengrass-testing
(if applicable)
Documentation Updates
docs/folderTesting
aws-greengrass-testing repository
Additional Notes
Any additional information, context, or screenshots that would be helpful for
reviewers.
By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.