-
Notifications
You must be signed in to change notification settings - Fork 485
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(command): publish device service response to external MQTT #4166
Conversation
Signed-off-by: Chris Hung <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just small name tweak for clarification
subscribe to device command response from device service via internal message bus and publish response back to 3rd-party requester Signed-off-by: Chris Hung <[email protected]>
Set(string, string) | ||
} | ||
|
||
func NewMessagingRouter() MessagingRouter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this Router determining which bus the response message goes to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad I forgot the most cirtical problem ...
A quick thought came to my mind is to maintain 2 maps internally :
func (r *MessagingRouter) Get(id string) (string, bool) {
topic, ok := r.ExternalMap[id]
if ok {
return topic, true
}
topic, _ = r.InternalMap[id]
return topic, false
}
func (r *MessagingRouter) Set(id string, topic string, source string) {
switch source {
case "internal":
r.InternalMap[id] = topic
case "external":
r.ExternalMap[id] = topic
}
}
Will try to improve it or find if there's better solution tomorrow ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that was what I had in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Save active requests in two maps using requestId as keys. This allows command service to know where to route the response (to external MQTT or internal MessageBus) Signed-off-by: Chris Hung <[email protected]>
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fix #4076
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:
describing the break)Testing Instructions
MESSAGEQUEUE_REQUIRED=true
environment variableedgex/command/request/<device-name>/<command-name>/<method>
topic. For example send following request toedgex/command/Simple-Device01/Xrotation/get
:edgex/command/response/<device-name>/<command-name>/<method>
topic:New Dependency Instructions (If applicable)