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.

Sign up for GitHub

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

Jump to bottom

Fix map #609

Merged
merged 15 commits into from
Jun 27, 2024
Merged

Fix map #609

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0ff8c7a
modified: src/v2i-hub/MapPlugin/manifest.json
jwillmartin Apr 17, 2024
f2f5d45
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Apr 18, 2024
2a9ff27
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Apr 18, 2024
64a451c
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Apr 18, 2024
1d2e0cc
Merge remote-tracking branch 'origin' into fix-MAP
jwillmartin Apr 18, 2024
d298723
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Apr 18, 2024
cfc8b43
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Apr 29, 2024
65a9e42
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin May 24, 2024
32dc37b
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin May 29, 2024
fce0eda
Merge remote-tracking branch 'origin' into fix-MAP
jwillmartin Jun 4, 2024
fa843d0
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Jun 12, 2024
aef5746
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Jun 13, 2024
7bbf823
modified: src/v2i-hub/MapPlugin/src/MapPlugin.h
jwillmartin Jun 13, 2024
bc83c6c
Merge branch 'develop' into fix-MAP
jwillmartin Jun 27, 2024
7de6bbd
modified: src/v2i-hub/MapPlugin/src/MapPlugin.cpp
jwillmartin Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/v2i-hub/MapPlugin/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
{
"key":"MAP_Files",
"default":"{ \"MapFiles\": [ {\"Action\":0, \"FilePath\":\"/var/www/plugins/MAP/MAP_9709_UPER.txt\"}] }",
"description":"JSON data defining a list of map files. One map file for each action set specified by the TSC."
"description":"JSON data defining a list of map files. One map file for each action set specified by the TSC."
}
]
}
222 changes: 90 additions & 132 deletions src/v2i-hub/MapPlugin/src/MapPlugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,114 +1,14 @@

#include <atomic>
#include <iostream>
#include <map>
#include <memory>
#include <mutex>
#include <sstream>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <thread>
#include <time.h>
#include <sys/time.h>

#include <tmx/tmx.h>
#include <tmx/IvpPlugin.h>
#include <tmx/messages/IvpBattelleDsrc.h>
#include <tmx/messages/IvpSignalControllerStatus.h>
#include <tmx/messages/IvpJ2735.h>
#include <tmx/j2735_messages/J2735MessageFactory.hpp>
#include "XmlMapParser.h"
#include "ConvertToJ2735r41.h"
#include "inputs/isd/ISDToJ2735r41.h"

#define USE_STD_CHRONO
#include <FrequencyThrottle.h>
#include <PluginClientClockAware.h>

#include "utils/common.h"
#include "utils/map.h"

#include <MapSupport.h>
using namespace std;
using namespace tmx;
using namespace tmx::messages;
using namespace tmx::utils;
#include "MapPlugin.h"

namespace MapPlugin {

#if SAEJ2735_SPEC < 63
UPERframe _uperFrameMessage;
#endif

class MapFile: public tmx::message {
public:
MapFile(): tmx::message() {}
virtual ~MapFile() {}

std_attribute(this->msg, int, Action, -1, );
std_attribute(this->msg, std::string, FilePath, "", );
std_attribute(this->msg, std::string, InputType, "", );
std_attribute(this->msg, std::string, Bytes, "", );
public:
static tmx::message_tree_type to_tree(MapFile m) {
return tmx::message::to_tree(static_cast<tmx::message>(m));
}

static MapFile from_tree(tmx::message_tree_type tree) {
MapFile m;
m.set_contents(tree);
return m;
}
};

//int _mapAction = -1;
//bool _isMapFilesNew = false;
//bool _isMapLoaded = false;

volatile int gMessageCount = 0;

class MapPlugin: public PluginClientClockAware {
public:
MapPlugin(string name);
virtual ~MapPlugin();

virtual int Main();
protected:
void UpdateConfigSettings();

// Virtual method overrides.
void OnConfigChanged(const char *key, const char *value);
void OnMessageReceived(IvpMessage *msg);
void OnStateChange(IvpPluginState state);

private:
std::atomic<int> _mapAction {-1};
std::atomic<bool> _isMapFileNew {false};
std::atomic<bool> _cohdaR63 {false};

std::map<int, MapFile> _mapFiles;
std::mutex data_lock;

J2735MessageFactory factory;

int sendFrequency = 1000;
FrequencyThrottle<int> errThrottle;

bool LoadMapFiles();
void DebugPrintMapFiles();
};

MapPlugin::MapPlugin(string name) :
PluginClientClockAware(name) {
AddMessageFilter(IVPMSG_TYPE_SIGCONT, "ACT", IvpMsgFlags_None);
SubscribeToMessages();
errThrottle.set_Frequency(std::chrono::minutes(30));
MapPlugin::MapPlugin(string name) : PluginClientClockAware(name) {
AddMessageFilter(IVPMSG_TYPE_SIGCONT, "ACT", IvpMsgFlags_None);
SubscribeToMessages();
errThrottle.set_Frequency(std::chrono::minutes(30));
}

MapPlugin::~MapPlugin() {

}
MapPlugin::~MapPlugin() {}

void MapPlugin::UpdateConfigSettings() {
GetConfigValue("Frequency", sendFrequency);
Expand Down Expand Up @@ -274,7 +174,7 @@ int MapPlugin::Main() {
msg->set_payload(byteStr);
msg->set_encoding(enc);
msg->set_flags(IvpMsgFlags_RouteDSRC);
msg->addDsrcMetadata(0x8002);
msg->addDsrcMetadata(tmx::messages::api::mapData_PSID);

activeAction = temp;
PLOG(logINFO) << "Map for action " << activeAction << " will be sent";
Expand Down Expand Up @@ -304,6 +204,39 @@ int MapPlugin::Main() {
return (EXIT_SUCCESS);
}

string MapPlugin::enum_to_hex_string()
{
sprintf(mapID_buffer, "%04X", tmx::messages::api::mapData);
string map_messageID = mapID_buffer;
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if this method is called and mapID_buffer is not initialized or garbage initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initialized with zeros in header file. Missed the {0}, but just added it.

return map_messageID;
}

string MapPlugin::removeMessageFrame(string &fileContent)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if this method is called and fileContent is not initialized or null initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fileContent is passed by reference and the method is only called after fileContent has already been initialized. Let me know if you want to add anything else to it to make sure this method doesn't crash the plugin

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be const string &filcontent

string map_messageID = enum_to_hex_string();

// Check for and remove MessageFrame
if (fileContent.size() >= 4 && fileContent.substr(0, 4) == map_messageID)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is a very robust way to determine whether the message contains the message frame or not. I think it would be more robust if we initially assumed the header was there and tried to decode it, if we run into an exception we log the exception as a warning and log that we are retrying without the header. Having code in our plugin that is couple to the exact format of the MAP message means that we have to rewrite it every-time the exact format changes. Having code that is loosely couple to just the C Struct and data means that the format can change without require logic changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could do that, but we'd still have to have logic to figure out whether the header has an additional 2 bytes or not. The message frame will always contain the message ID, at the beginning so this is a good way to quickly check for it instead of trying and failing to encode the message. I think both ways would work, but I'd rather run this check instead of attempting a blind encoding.

// Check if message is hex size > 255, remove appropriate header
string tempFrame = fileContent;
tempFrame.erase(0, 6);
PLOG(logDEBUG4) << "Checking size of: " << tempFrame;
if (tempFrame.size() > 510)
{
fileContent.erase(0, 8);

}
else
{
fileContent.erase(0, 6);
}

PLOG(logDEBUG4) << "Payload without MessageFrame: " << fileContent;
}
return fileContent;
}

bool MapPlugin::LoadMapFiles()
{
if (_mapFiles.empty())
Expand Down Expand Up @@ -342,43 +275,68 @@ bool MapPlugin::LoadMapFiles()
else if (inType == "TXT")
{
byte_stream bytes;
ifstream in(fn);
in >> bytes;

PLOG(logINFO) << fn << " MAP encoded bytes are " << bytes;

MapDataMessage *mapMsg = MapDataEncodedMessage::decode_j2735_message<codec::uper<MapDataMessage> >(bytes);
if (mapMsg) {
PLOG(logDEBUG) << "Map is " << *mapMsg;
std::ifstream in(fn, std::ios::binary);
if (!in)
{
PLOG(logERROR) << "Failed to open file: " << fn;
}
else
{
try
{
string fileContent((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
in.close();
// Remove any newline characters
fileContent.erase(remove(fileContent.begin(), fileContent.end(), '\n'), fileContent.end());
PLOG(logDEBUG4) << "Map without newline " << fileContent;
fileContent = removeMessageFrame(fileContent);

std::istringstream streamableContent(fileContent);
streamableContent >> bytes;
PLOG(logINFO) << fn << " MAP encoded bytes are " << bytes;
MapDataMessage *mapMsg = MapDataEncodedMessage::decode_j2735_message<codec::uper<MapDataMessage>>(bytes);

if (mapMsg)
{
PLOG(logDEBUG) << "Map is " << *mapMsg;

MapDataEncodedMessage mapEnc;
mapEnc.encode_j2735_message(*mapMsg);
mapFile.set_Bytes(mapEnc.get_payload_str());
MapDataEncodedMessage mapEnc;
mapEnc.encode_j2735_message(*mapMsg);
mapFile.set_Bytes(mapEnc.get_payload_str());

PLOG(logINFO) << fn << " J2735 message bytes encoded as " << mapFile.get_Bytes();
PLOG(logINFO) << fn << " J2735 message bytes encoded as " << mapFile.get_Bytes();
}
} catch (const std::ios_base::failure& e)
{
PLOG(logERROR) << "Exception encountered while reading file: " << e.what();
}
}
}
else if (inType == "UPER")
{
PLOG(logDEBUG) << "Reading MAP file as UPER encoded hex bytes including MessageFrame." << std::endl;
std::ifstream in;
PLOG(logDEBUG) << "Reading MAP file as UPER encoded hex bytes including MessageFrame.";
std::ifstream in;
try {
in.open(fn, std::ios::in | std::ios::binary );
if (in.is_open()) {
if (in.is_open())
{
in.seekg(0, std::ios::end);
int fileSize = in.tellg();
in.seekg(0, std::ios::beg);
PLOG(logDEBUG) << "File size is " << fileSize <<std::endl;
std::string bytes_string((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
PLOG(logDEBUG) << "File contents : " << bytes_string << std::endl;
mapFile.set_Bytes(bytes_string);
PLOG(logDEBUG) << "File size is: " << fileSize;
string bytes_string((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
bytes_string.erase(remove(bytes_string.begin(), bytes_string.end(), '\n'), bytes_string.end());
PLOG(logDEBUG) << "File contents: " << bytes_string;
mapFile.set_Bytes(bytes_string);
}
else {
PLOG(logERROR) << "Failed to open file " << fn << "." << std::endl;
else
{
PLOG(logERROR) << "Failed to open file: " << fn << ".";
}
}
catch( const ios_base::failure &e) {
PLOG(logERROR) << "Exception Encountered : \n" << e.what();
catch( const ios_base::failure &e)
{
PLOG(logERROR) << "Exception Encountered: \n" << e.what();
}
}
else if (inType == "XML")
Expand All @@ -396,7 +354,7 @@ bool MapPlugin::LoadMapFiles()
mapEnc.encode_j2735_message(mapMsg);
mapFile.set_Bytes(mapEnc.get_payload_str());

PLOG(logINFO) << fn << " XML file encoded as " << mapFile.get_Bytes();
PLOG(logINFO) << fn << " XML file encoded as: " << mapFile.get_Bytes();
}
else
{
Expand All @@ -421,7 +379,7 @@ bool MapPlugin::LoadMapFiles()

mapFile.set_Bytes(mapEnc->get_payload_str());

PLOG(logINFO) << fn << " input file encoded as " << mapEnc->get_payload_str();
PLOG(logINFO) << fn << " input file encoded as: " << mapEnc->get_payload_str();
}
else
{
Expand Down
109 changes: 109 additions & 0 deletions src/v2i-hub/MapPlugin/src/MapPlugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
#pragma once
Copy link
Contributor

Choose a reason for hiding this comment

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

#pragma once

is a way to guard against against headers being included multiple times. This inclusion guard below does the same thing

#ifndef MAPPLUGIN_H_
#define MAPPLUGIN_H_

There is no reason to do both. Just pick one (https://learn.microsoft.com/en-us/cpp/preprocessor/once?view=msvc-170)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kept is as #pragma once, thanks!


#include <atomic>
#include <iostream>
#include <map>
#include <memory>
#include <mutex>
#include <sstream>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <thread>
#include <time.h>
#include <sys/time.h>
#include <string>
#include <cstdio>
#include <chrono>
#include <fstream>
#include <boost/algorithm/string.hpp>

#include <tmx/tmx.h>
#include <tmx/IvpPlugin.h>
#include <tmx/messages/IvpBattelleDsrc.h>
#include <tmx/messages/IvpSignalControllerStatus.h>
#include <tmx/messages/IvpJ2735.h>
#include <tmx/j2735_messages/J2735MessageFactory.hpp>
#include <tmx/TmxApiMessages.h>
#include "XmlMapParser.h"
#include "ConvertToJ2735r41.h"
#include "inputs/isd/ISDToJ2735r41.h"

#define USE_STD_CHRONO
#include <FrequencyThrottle.h>
#include <PluginClientClockAware.h>

#include "utils/common.h"
#include "utils/map.h"

#include <MapSupport.h>

using namespace std;
using namespace tmx;
using namespace tmx::messages;
using namespace tmx::utils;

namespace MapPlugin {

#if SAEJ2735_SPEC < 63
UPERframe _uperFrameMessage;
#endif

class MapFile: public tmx::message {
public:
MapFile(): tmx::message() {}
virtual ~MapFile() {}

std_attribute(this->msg, int, Action, -1, );
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the rest of actions this MAP support? Which default action is this initialized to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default is no action existing. It looks like only action is for finding the file path.

std_attribute(this->msg, std::string, FilePath, "", );
std_attribute(this->msg, std::string, InputType, "", );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this "inputType" used? What does this represent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it was declared but is expected to be empty? I'll find out if we need it or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's needed to find the file extension type for ISD, TXT, XML, JSON, UPER. Kept it in

Copy link
Collaborator

@dan-du-car dan-du-car Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Cool. What is the difference between UPER and TXT upload? It seems they both upload bytes but TXT has message frame in it? Why do we need these two different uploads that has similar content? I am not familiar with the usage of this plugin, so I am a bit curious about these different extension types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UPER is used to accept the message frame, but it assumes the message is correct and broadcasts it to TMX core.
I made changes to the TXT to accept the message frame, but it strips it and decodes the remaining content to verify the message

Copy link
Contributor

Choose a reason for hiding this comment

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

I think any format we accept should very message content before broadcast. When it is not verified we get exceptions on down stream plugins that attempt to ingest this message. I created the TXT format a while back to avoid dealing with issues with the UPER format logic. These two formats can likely be consolidated since they are both uper hex

std_attribute(this->msg, std::string, Bytes, "", );

static tmx::message_tree_type to_tree(MapFile m) {
return tmx::message::to_tree(static_cast<tmx::message>(m));
}

static MapFile from_tree(tmx::message_tree_type tree) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these from_tree and to_tree functions used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't look like it. They were also included in another the inputs/ISDDataAdaptor.hpp and also not used. I'll remove it and retest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was needed by message.hpp in TmxApi. Left it in

MapFile m;
m.set_contents(tree);
return m;
}
};

class MapPlugin: public PluginClientClockAware {
public:
MapPlugin(string name);
virtual ~MapPlugin();
virtual int Main();

protected:
void UpdateConfigSettings();

// Virtual method overrides.
void OnConfigChanged(const char *key, const char *value);
void OnMessageReceived(IvpMessage *msg);
void OnStateChange(IvpPluginState state);

private:
std::atomic<int> _mapAction {-1};
std::atomic<bool> _isMapFileNew {false};
std::atomic<bool> _cohdaR63 {false};

std::map<int, MapFile> _mapFiles;
std::mutex data_lock;

J2735MessageFactory factory;

int sendFrequency = 1000;
FrequencyThrottle<int> errThrottle;

char mapID_buffer[5] = {0};

bool LoadMapFiles();
void DebugPrintMapFiles();
string enum_to_hex_string();
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to convert the messageID from a decimal enum to a hex string. The only place the message ID is declared in TmxApiMessages

string removeMessageFrame(string &fileContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Left an earlier comment about how I think this would be better implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Responded in other comment


};

} // namespace MapPlugin
Loading