Skip to content

feat: integrate InfluxDB telemetry writer from cpp-tester#8

Open
Fangrui-Zhang wants to merge 1 commit intomainfrom
cpp-tester-simulation
Open

feat: integrate InfluxDB telemetry writer from cpp-tester#8
Fangrui-Zhang wants to merge 1 commit intomainfrom
cpp-tester-simulation

Conversation

@Fangrui-Zhang
Copy link
Copy Markdown
Contributor

  • Copy InfluxWriter, CsvParser, TelemetryRecord from sc2-telemetry-tester
  • Add buildTelemetryRecord() to DataUnpacker to map live car data to TelemetryRecord
  • Wire InfluxWriter into main loop to publish telemetry every 2 seconds
  • Update CMakeLists.txt: add libcurl, influx sources and include paths

- Copy InfluxWriter, CsvParser, TelemetryRecord from sc2-telemetry-tester
- Add buildTelemetryRecord() to DataUnpacker to map live car data to TelemetryRecord
- Wire InfluxWriter into main loop to publish telemetry every 2 seconds
- Update CMakeLists.txt: add libcurl, influx sources and include paths
Copilot AI review requested due to automatic review settings March 22, 2026 20:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Integrates an InfluxDB telemetry publishing path into the main SC2 driver IO loop by introducing an InfluxWriter (libcurl) and a TelemetryRecord mapping from live DataUnpacker state.

Changes:

  • Added InfluxWriter, CsvParser, and TelemetryRecord sources/headers under influx/.
  • Added DataUnpacker::buildTelemetryRecord() and wired periodic publishing (every ~2s) in main.cpp.
  • Updated CMakeLists.txt to find/link libcurl and include the new influx sources/headers.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
main.cpp Initializes InfluxWriter from env and periodically publishes telemetry.
data_processor/dataUnpacker.h Declares buildTelemetryRecord() and includes TelemetryRecord.
data_processor/dataUnpacker.cpp Implements mapping from live unpacked values to TelemetryRecord.
influx/include/TelemetryRecord.h Defines the telemetry schema struct used for line protocol encoding.
influx/include/InfluxWriter.h Declares Line Protocol conversion and write helpers.
influx/src/InfluxWriter.cpp Implements Line Protocol formatting and HTTP POST via libcurl.
influx/include/CsvParser.h Declares CSV-to-TelemetryRecord parser (tester utility).
influx/src/CsvParser.cpp Implements CSV parsing and header→field mapping.
influx/src/tester_main.cpp Adds a standalone CSV→Influx test driver.
CMakeLists.txt Adds CURL dependency + influx sources/includes to the build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +212 to +219
// ── Init InfluxDB writer ─────────────────────────────────────────────────
InfluxWriter writer;
try {
// InfluxWriter constructor throws if env vars are missing
} catch (const std::exception& ex) {
std::cerr << "[error] InfluxWriter init failed: " << ex.what() << "\n";
return 1;
}
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

InfluxWriter writer is constructed before the try/catch block, so any exception thrown by the constructor won’t be caught here (the catch is effectively dead code). Construct the InfluxWriter inside the try block (or use an optional/unique_ptr) so missing env vars are handled as intended.

Copilot uses AI. Check for mistakes.
bool CsvParser::toBool(const std::string& s, bool fallback) {
if (s.empty()) return fallback;
std::string lower = s;
std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower);
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

CsvParser::toBool uses ::tolower on a plain char via std::transform, which is undefined behavior for negative char values. Use an unsigned-char cast (or a lambda that casts to unsigned char) before calling std::tolower.

Suggested change
std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower);
std::transform(lower.begin(), lower.end(), lower.begin(),
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +16
TelemetryRecord DataUnpacker::buildTelemetryRecord() const {
TelemetryRecord r;

// ── MCC / Motor Control ──────────────────────────────────────────────────
r.accelerator_pedal = getAcceleratorPedal();
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

buildTelemetryRecord() reads many DataUnpacker fields (including the cell_group_voltages vector) without taking the class mutex, while unpack() writes under mutex. This can cause data races and inconsistent snapshots. Consider locking the same mutex while building the record (you may need to make the mutex mutable or make buildTelemetryRecord non-const).

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +54
target_link_libraries(${PROJECT_NAME} PRIVATE CURL::libcurl)

target_include_directories(${PROJECT_NAME} PRIVATE ./ influx/include)
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

CMakeLists.txt sets cmake_minimum_required(VERSION 3.5) but links with the imported target CURL::libcurl, which is not available in older CMake FindCURL implementations. Either raise the minimum CMake version to one that guarantees CURL::libcurl, or link using the legacy CURL_LIBRARIES/CURL_INCLUDE_DIRS variables.

Suggested change
target_link_libraries(${PROJECT_NAME} PRIVATE CURL::libcurl)
target_include_directories(${PROJECT_NAME} PRIVATE ./ influx/include)
target_link_libraries(${PROJECT_NAME} PRIVATE ${CURL_LIBRARIES})
target_include_directories(${PROJECT_NAME} PRIVATE ./ influx/include ${CURL_INCLUDE_DIRS})

Copilot uses AI. Check for mistakes.
Comment on lines +313 to +318
curl_easy_setopt(curl, CURLOPT_URL, endpoint.c_str());
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, body.c_str());
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, static_cast<long>(body.size()));
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, curlWriteCallback);
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &responseBody);
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

InfluxWriter::httpPost has no connect/overall timeout set on the CURL handle, so a network stall can block the caller thread indefinitely (and in this app, stall the main loop). Consider setting CURLOPT_CONNECTTIMEOUT_MS/CURLOPT_TIMEOUT_MS (and CURLOPT_NOSIGNAL for multi-threaded safety) and optionally doing a one-time curl_global_init/cleanup in the process lifetime.

Copilot uses AI. Check for mistakes.
@@ -5,6 +5,7 @@
#include <cstdlib>
#include <signal.h>
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

main.cpp uses std::unique_ptr/std::make_unique but does not include , which will fail to compile on standard-conforming toolchains. Add the missing standard header include near the other includes.

Suggested change
#include <signal.h>
#include <signal.h>
#include <memory>

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,62 @@
#pragma once
#include <string>
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

InfluxWriter.h declares writeBatch(const std::vectorstd::string&) but doesn’t include , so any translation unit including this header can fail to compile. Include in the header (don’t rely on transitive includes).

Suggested change
#include <string>
#include <string>
#include <vector>

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants