-
Notifications
You must be signed in to change notification settings - Fork 96
Add MQTT Entity Sink Feature: Flat Topic Streaming feat: #535 #571
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
Conversation
|
I'll review the code and test locally. |
|
Reviewing issues with build in CI/CD process. Creating new branch on cppagent for testing. |
issues probably with the newly added test file mqtt_entity_sink_test.cpp or the underlying logic in mqtt_entity_sink. a bad use of shared pointers? I am currently not able to figure out the issue. a robust review and some code update will be helpful. Feel free to make any changes necessary. |
|
There were a few issues I can help you fix. I’ll send some more comments soon.
|
|
I have created a patch file that should fix the issue with the tests. The issue was the client was being closed after the test had exited and the local objects were not there. I just added a method to close the client before the function exits. I also ran it through clangformat, so some of the changes are just whitespace related. |
|
There are still some issues. Using a heap sanitizer and allocation debugger to test. |
|
Fixed another timing issue when checking for unavailable to exclude conditions and the agent device. |
|
I created a local branch: feature/mqtt-entity-sink if you want to merge against a branch. |
wsobel
left a comment
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. If it passes, I'll merge and update the version.
|
|
||
| ### SAMPLE Observation | ||
| ```json | ||
| { |
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.
Did you try the json entity printer that generates a similar representation for the observations. It also supports all the extended types like data sets and tables. I didn't see that code in the formatter.
What are your thoughts?
| } | ||
|
|
||
| void TearDown() override { | ||
| if (m_client) { |
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.
You need to shut the agent down first. Otherwise there are issues. I can send patch.
| const std::string& topic, | ||
| const std::string& payload) { | ||
| receivedJson = json::parse(payload); | ||
| if (receivedJson.contains("name")) |
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.
The callback is called during cleanup of the client. You need to either shut down the client or remove the handler before exiting this test. Otherwise the json object has been destructed and is no longer viable.
Adds feature #535
This pull request addresses #535 and introduces the MQTT Entity Sink feature to the cppagent, enabling observations to MQTT brokers using a flat, per-data-item topic structure.
Key changes include:
MqttEntitySinkclass and integration with agent configuration.Checklist
Future development directions:
@wsobel