-
Notifications
You must be signed in to change notification settings - Fork 68
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
Telematic bridge nats #567
Changes from 43 commits
2cb49ca
9b685ae
d9dddf9
614941b
4b4a8b8
f21faab
0fac3f8
47de4e6
30d8528
eece9af
b9d209e
8b4d24d
5ca81e0
a3b68aa
2f951d4
3c6143c
9e63a6a
31f0be3
bebc386
dccd1be
7107404
f445eeb
e38ff04
7c050bd
c72ecc9
383bafd
758a8ea
6175f1f
d3936df
4370816
2d6364a
8b01468
466db6b
9ce48c0
3e58116
9b4e478
fbe124c
e867876
986b979
4278be8
71ac033
1c59a3e
d5998cf
040a99b
bf290f5
e1d7315
ebaa421
1fee071
691178f
9d89744
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
## Telematic bridge introduction | ||
Telematic bridge is a V2xHub plugin used to collect data stream generated by V2xHub and send the data stream into [telematic tool](https://github.com/usdot-fhwa-stol/cda-telematics) hosted in the AWS cloud. | ||
## NATS Publisher/Subscriber | ||
### NATS Connections and registration | ||
#### Telematic plugin sends registration request to telematic server | ||
##### Subject | ||
NATS subject: *.register_unit | ||
##### Request | ||
``` | ||
{ | ||
"unit_id": "<unit_id>" | ||
} | ||
``` | ||
##### Response | ||
``` | ||
{ | ||
"unit_id": "<unit_id>", | ||
"unit_type": "infrastructure", | ||
"unit_name": "East Intersection", | ||
"timestamp": "1678998191815233965", | ||
"event_name": "wfd_integration_testing", | ||
"location": "TFHRC", | ||
"testing_type": "Integration" | ||
} | ||
``` | ||
|
||
### Available topics | ||
#### Telematic plugin receives request from telematic UI | ||
##### Subject | ||
NATS subject: <unit_id>.available_topics | ||
##### Request | ||
``` | ||
{ | ||
"unit_id": "<unit_id>" | ||
} | ||
``` | ||
##### Reply | ||
|
||
``` | ||
{ | ||
"unit_id": "<unit_id>", | ||
"unit_type": "infrastructure", | ||
"unit_name": "East Intersection", | ||
"timestamp": "1678998191815233965", | ||
"event_name": "wfd_integration_testing", | ||
"location": "TFHRC", | ||
"testing_type": "Integration", | ||
"topics": [ | ||
{ | ||
"name": "J2735_TMSG03-P_CARMAStreetsPlugin" | ||
}, | ||
{ | ||
"name": "<topic_name_2>" | ||
} | ||
] | ||
} | ||
``` | ||
|
||
### Selected topics | ||
#### Telematic plugin receives selected topics from telematic UI | ||
##### Subject | ||
NATS subject: <unit_id>.publish_topics | ||
#### Request | ||
``` | ||
{ | ||
"unit_id": "<unit_id>", | ||
"unit_type": "infrastructure", | ||
"timestamp": 1663084528513000400, | ||
"event_name": "wfd_integration_testing", | ||
"location": "TFHRC", | ||
"testing_type": "Integration", | ||
"topics": [ | ||
"<topic_name_1>", | ||
"<topic_name_2>" | ||
] | ||
} | ||
``` | ||
##### Reply | ||
``` | ||
"request received!" | ||
``` | ||
|
||
## Check status | ||
#### Telematic plugin receives live status request from telematic server | ||
##### Subject | ||
NATS subject: <unit_id>.check_status | ||
##### Request | ||
``` | ||
{ | ||
"unit_id": "<unit_id>" | ||
} | ||
``` | ||
##### Reponse | ||
``` | ||
"OK" | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,26 @@ | |
"key": "LogLevel", | ||
"default": "INFO", | ||
"description": "The log level for this plugin" | ||
}, | ||
{ | ||
"key": "NATSUrl", | ||
"default": "nats://127.0.0.1:4222", | ||
"description": "The NATS connection URL" | ||
}, | ||
{ | ||
"key": "UnitId", | ||
"default": "v2xhub_id", | ||
"description": "The unique identifier for a telematic unit" | ||
}, | ||
{ | ||
"key": "UnitName", | ||
"default": "East Intersection", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this match J2735 intersection name or does it need to be separate. If it can match I suggest we just add in the description to set this to intersection name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed 1c59a3e There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please specify J2735 MAP in the description. So should be set to MAP J2735 intersection name if available. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not specific about MAP message nor intersection. If we deploy this v2xhub in Cav-in-a-box at roadside, there is no concept of intersection. This east intersection is just a default value I give cause I test it with east intersection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is not MAP Message specific from an implementation stand point but we should not be creating unnecessary new IDs in our different pieces of functionality. For example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we table this for now? The unit id is something we used in the telematic tool context. The same design for this unit id is applied to carma-streets, carma-platform (vehicle), and carma-cloud. If this has some conflicts with IDs in a simulated environment, we probably want to decide and make the changes to all units to better fit the simulated environment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have unique IDs for carma streets, carma-cloud , carma-platform as well to register for the simulation? |
||
"description": "A descriptive name for each telematic unit. It can also be the intersection name where this unit collect data from." | ||
}, | ||
{ | ||
"key": "MessageExclusionList", | ||
"default": "System_KeepAlive_CommandPlugin,System_KeepAlive_CARMAStreetsPlugin,System_KeepAlive_CDASimAdapter,System_KeepAlive_MessageReceiver", | ||
"description": "The list of messages are excluded from the available message list. Message name is a combination of message type, subtype and source separated by underscore. E.G: type_subtype_source" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,14 @@ | ||
#include "TelematicBridgePlugin.h" | ||
|
||
using namespace tmx::utils; | ||
using namespace std; | ||
|
||
namespace TelematicBridge | ||
{ | ||
TelematicBridgePlugin::TelematicBridgePlugin(const string &name) : PluginClient(name) | ||
{ | ||
_telematicUnitPtr = make_shared<TelematicUnit>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be a unique pt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason for this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is best practice to limit pointer scope or reference scope where possible to avoid other users possibly unintentionally impacting the lifecycle of this pointer by holding a reference. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This point is shared with this plugin. I cannot find a case where users will accidentally change the lifecycle of this pointer. This pointer should be sharing the same lifecycle of the plugin itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the plugin has a reference to this object but the telematics bridge is no longer in scope, the telematics bridge will still not get deleted because the plugin holds a reference to this pointer which will prevent the pointer from getting deleted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the plugin need a reference to this pointer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So that the plugin can use this pointer to send data into the telematic tool. It does not make sense for this plugin to subscribe messages but not forward them into the telematic tool. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paulbourelly999 Addressed 040a99b |
||
UpdateConfigSettings(); | ||
AddMessageFilter("*", "*", IvpMsgFlags_None); | ||
AddMessageFilter("J2735", "*", IvpMsgFlags_RouteDSRC); | ||
SubscribeToMessages(); | ||
|
@@ -20,14 +25,55 @@ namespace TelematicBridge | |
auto messageFm = (MessageFrame_t *)calloc(1, sizeof(MessageFrame_t)); | ||
DecodeJ2735Msg(msg->payload->valuestring, messageFm); | ||
string xml_payload_str = ConvertJ2735FrameToXML(messageFm); | ||
ASN_STRUCT_FREE(asn_DEF_MessageFrame, messageFm); | ||
json["payload"] = StringToJson(xml2Json(xml_payload_str)); | ||
ASN_STRUCT_FREE(asn_DEF_MessageFrame, messageFm); | ||
string json_payload_str = xml2Json(xml_payload_str.c_str()); | ||
json["payload"] = StringToJson(json_payload_str); | ||
} | ||
|
||
auto jsonStr = JsonToString(json); | ||
PLOG(logINFO) << jsonStr; | ||
stringstream topic; | ||
topic << (msg->type ? msg->type : "") << "_" << (msg->subtype ? msg->subtype : "") << "_" << (msg->source ? msg->source : ""); | ||
auto topicStr = topic.str(); | ||
_telematicUnitPtr->updateAvailableTopics(topicStr); | ||
if (_telematicUnitPtr->inSelectedTopics(topicStr)) | ||
{ | ||
_telematicUnitPtr->publishMessage(topicStr, json); | ||
} | ||
} | ||
} | ||
|
||
void TelematicBridgePlugin::UpdateConfigSettings() | ||
{ | ||
lock_guard<mutex> lock(_configMutex); | ||
GetConfigValue<string>("NATSUrl", _natsURL); | ||
GetConfigValue<string>("UnitId", _unitId); | ||
GetConfigValue<string>("UnitName", _unitName); | ||
GetConfigValue<string>("MessageExclusionList", _excludedMessages); | ||
unit_st unit = {_unitId, _unitName, UNIT_TYPE_INFRASTRUCTURE}; | ||
if (_telematicUnitPtr) | ||
{ | ||
_telematicUnitPtr->setUnit(unit); | ||
_telematicUnitPtr->updateExcludedTopics(_excludedMessages); | ||
} | ||
} | ||
|
||
void TelematicBridgePlugin::OnStateChange(IvpPluginState state) | ||
{ | ||
PluginClient::OnStateChange(state); | ||
if (state == IvpPluginState_registered) | ||
{ | ||
UpdateConfigSettings(); | ||
if (_telematicUnitPtr) | ||
{ | ||
_telematicUnitPtr->connect(_natsURL); | ||
} | ||
} | ||
} | ||
|
||
void TelematicBridgePlugin::OnConfigChanged(const char *key, const char *value) | ||
{ | ||
PluginClient::OnConfigChanged(key, value); | ||
UpdateConfigSettings(); | ||
} | ||
} | ||
|
||
// The main entry point for this application. | ||
|
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.
We are currently, in CDASim pulling and infrastructure ID from an environment variable. You may want to do the same here. See CDASimAdapter
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.
This is not the same concept as the infrastructure id. It is a unique identifier we give to a telematic unit to distinguish it from other units like streets and vehicle. Is there any rule when to use the UI configuration parameter and when to use the environment variables? All these parameters make sense when they are together.
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.
Hmm, so part of the UC for this plugin is to disable health of different RSUs, and therefore infrastructure instances. Coming up with and ID that is completely different from the J2735 intersection ID and our infrastructure ID I think is bad practice. Then we will have 3 different IDs for the same instance, all which are completely indepedent.
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.
I think this unit id value can be anything that makes sense to end user during their testing with v2xhub as long as it is unique in the telematic tool.
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.
So then why don't we just re-use INFRASTRUCTURE ID here? The same environment variable we use for CDASimAdapter.
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.
What is the value of the infrastructure id? We also display this ID in the UI. Does it make sense to end users to use infrastructure id like 1079? Or use a more descriptive unit id, like east_intersection_1079 or v2xhub_1079?
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.
Something like <rsu_21312>. We have not standardized this yet but I think if we say it has to be rsu_ then maybe we can minimize different ID
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 unit id is under the telematic tool context. They may share the same value. For debugging and ease of usability purpose, it might be more obvious if we keep both unit name and unit id in the same place like here on the V2xHub UI.