465 lines
30 KiB
Markdown
465 lines
30 KiB
Markdown
╭─── Claude Code v2.1.42 ────────────────────────────────────────────────────────────────────────────────╮
|
||
│ │ Tips for getting started │
|
||
│ Welcome back! │ Run /init to create a CLAUDE.md file with instructions for … │
|
||
│ │ ──────────────────────────────────────────────────────────── │
|
||
│ │ Recent activity │
|
||
│ ▐▛███▜▌ │ No recent activity │
|
||
│ ▝▜█████▛▘ │ │
|
||
│ ▘▘ ▝▝ │ │
|
||
│ kimi-k2.5:cloud · API Usage Billing │ │
|
||
│ ~/projekty/modborno3 │ │
|
||
╰────────────────────────────────────────────────────────────────────────────────────────────────────────╯
|
||
|
||
/model to try Opus 4.6
|
||
|
||
❯ analyse modb_orno3.c
|
||
|
||
● Read 1 file (ctrl+o to expand)
|
||
|
||
● Analysis of modb_orno3.c
|
||
|
||
Overview
|
||
|
||
This is a C program that reads energy data from two Modbus RTU devices:
|
||
- ORNO - 3-phase energy meter (voltage, current, power, energy, frequency)
|
||
- SUN2000 - Huawei solar inverter
|
||
|
||
Data is published to MQTT (using Eclipse Mosquitto) and InfluxDB v2.
|
||
|
||
---
|
||
Architecture
|
||
|
||
Modbus Communication
|
||
- Uses libmodbus for RTU serial communication via /dev/ttyUSB0
|
||
- ORNO: slave address 2, even parity ('E'), RTS delay 5ms
|
||
- SUN2000: slave address 3, no parity ('N'), RTS delay 5ms
|
||
- Timing parameters configurable via #define directives (lines 41-50)
|
||
|
||
Data Flow
|
||
┌─────────────┐ ┌─────────────┐ ┌─────────────┐
|
||
│ ORNO │ │ SUN2000 │ │ Output │
|
||
│ (meter) │ │ (inverter) │ │ │
|
||
└──────┬──────┘ └──────┬──────┘ ├─> MQTT │
|
||
│ │ ├─> InfluxDB │
|
||
└───────┬───────────┘ │ (HTTP) │
|
||
│ └─────────────┘
|
||
modbus_read_registers()
|
||
|
||
---
|
||
Key Features
|
||
|
||
1. Voltage Fluctuation Detection (lines 546-572)
|
||
- Circular buffer of 12 samples tracks voltage history
|
||
- Detects fluctuations >5% from average
|
||
- Triggers high-frequency mode (300s duration, 10ms polling)
|
||
2. Spike Detection (lines 591-649)
|
||
- Validates readings against previous values
|
||
- Rejects changes >80% (factor of 1.8x) for voltage, current, power
|
||
- Rejects energy decreases (line 630)
|
||
- Frequency validated within ±5%
|
||
3. Range Validation (lines 360-375)
|
||
- Checks for NaN/Inf values
|
||
- Validates against min/max thresholds (e.g., voltage 150-280V)
|
||
|
||
---
|
||
Register Mappings
|
||
|
||
ORNO registers (lines 128-140):
|
||
- Voltages: 0x0E, 0x10, 0x12 (L1, L2, L3)
|
||
- Currents: 0x16 (start)
|
||
- Power: 0x1C (total + per-phase)
|
||
- Energy: 0x100 (total + per-phase)
|
||
- Frequency: 0x14
|
||
|
||
SUN2000 registers (lines 105-126):
|
||
- Startup/shutdown times, device status, alarms
|
||
- PV input power, phase voltages, peak/active/reactive power
|
||
- Grid frequency, temperature, energy yields
|
||
|
||
---
|
||
Security Concerns
|
||
|
||
Critical: Line 65 contains a hardcoded InfluxDB token:
|
||
#define INFLUX_TOKEN
|
||
"BCIZ6kcCAVgpcwSfU0PBS7m0Zb6an93kuOtizbEtlXub-uaoYp4dmIQBQYaJCv8_KE4QYYZ08jxtpaZ3TUWP-Q=="
|
||
|
||
Other issues:
|
||
- Hardcoded MQTT broker (10.1.1.1:1883) - no TLS
|
||
- InfluxDB connection uses plaintext HTTP (port 5086)
|
||
- No authentication on MQTT (no username/password)
|
||
|
||
---
|
||
Code Quality Observations
|
||
|
||
Strengths:
|
||
- Good error handling for Modbus operations
|
||
- Timing instrumentation (modbus_read_timed())
|
||
- Previous-value fallback on read failures
|
||
- Configurable via compile-time defines
|
||
|
||
Weaknesses:
|
||
- Uses sprintf() without bounds checking (potential buffer overflow)
|
||
- No free(buf) in mosq_test() (line 408) - memory leak
|
||
- Mixed Polish/English in comments
|
||
- mosq_test() declared to return int but has no return statement
|
||
- main() uses do-while with exit(1) on errors - connection leaks on ORNO failure
|
||
|
||
---
|
||
Potential Bugs
|
||
|
||
1. Line 630 - Energy spike detection logic:
|
||
if (prev_energy.W_Tot != 0 && (current_energy.W_Tot > prev_energy.W_Tot * 1.8 || current_energy.W_Tot <
|
||
prev_energy.W_Tot))
|
||
1. This triggers on ANY decrease in energy (not just spikes), which may be intentional for a
|
||
non-decreasing counter.
|
||
2. Line 565-567 - Division by zero risk if voltage_buffer_items is 0 (protected by check on line 551,
|
||
but edge case exists).
|
||
3. Line 797 - asctime() returns pointer to static buffer - not thread-safe.
|
||
4. Line 303 - strlen(data) with potential unterminated string if influx_send_* functions receive bad
|
||
input.
|
||
|
||
---
|
||
Recommendations
|
||
|
||
1. Move credentials to environment variables or config file
|
||
2. Add TLS support for MQTT and InfluxDB
|
||
3. Replace sprintf() with snprintf()
|
||
4. Add signal handling for clean shutdown
|
||
5. Consider using modbus_read_registers() batch reads for SUN2000 to reduce round-trips
|
||
|
||
✻ Sautéed for 55s
|
||
✻ Sautéed for 55s
|
||
|
||
❯ /model
|
||
⎿ Set model to opus (kimi-k2.5:cloud)
|
||
|
||
❯ analyse modb_orno3.c
|
||
|
||
● I already have the file content from our previous conversation. Here's a comprehensive analysis:
|
||
|
||
Code Analysis: modb_orno3.c
|
||
|
||
Purpose
|
||
|
||
A Modbus RTU data collector that reads energy measurements from two devices:
|
||
- ORNO (3-phase electricity meter) - voltages, currents, power, energy, frequency
|
||
- SUN2000 (Huawei solar inverter) - status, alarms, PV power, temperatures, yields
|
||
|
||
Publishes data to MQTT (Eclipse Mosquitto) and InfluxDB v2.
|
||
|
||
---
|
||
Architecture
|
||
|
||
┌─────────────┐ ┌──────────────┐ ┌─────────────┐
|
||
│ ORNO │ │ Program │ │ MQTT │
|
||
│ (addr 2) │─────>│ modb_orno3 │─────>│ Broker │
|
||
│ 9600,E,8,1 │ │ │ │ 10.1.1.1 │
|
||
└─────────────┘ │ │ └─────────────┘
|
||
│ │
|
||
┌─────────────┐ │ │ ┌─────────────┐
|
||
│ SUN2000 │ │ │ │ InfluxDB │
|
||
│ (addr 3) │─────>│ │─────>│ 10.1.1.1 │
|
||
│ 9600,N,8,1 │ └──────────────┘ │ :5086 │
|
||
└─────────────┘ └─────────────┘
|
||
|
||
---
|
||
Key Features
|
||
|
||
┌─────────────────────┬────────────────────────────────────────────────────────────────────────┐
|
||
│ Feature │ Implementation │
|
||
├─────────────────────┼────────────────────────────────────────────────────────────────────────┤
|
||
│ Spike Detection │ Rejects values changing >80% (1.8x factor) for U/I/P │
|
||
├─────────────────────┼────────────────────────────────────────────────────────────────────────┤
|
||
│ Voltage Fluctuation │ Circular buffer (12 samples), triggers high-freq mode on >5% deviation │
|
||
├─────────────────────┼────────────────────────────────────────────────────────────────────────┤
|
||
│ Data Validation │ NaN/Inf checks, min/max range validation │
|
||
├─────────────────────┼────────────────────────────────────────────────────────────────────────┤
|
||
│ Retry Logic │ Up to 10 retries per SUN2000 register │
|
||
├─────────────────────┼────────────────────────────────────────────────────────────────────────┤
|
||
│ Timed Reads │ Logs Modbus operation duration │
|
||
└─────────────────────┴────────────────────────────────────────────────────────────────────────┘
|
||
|
||
---
|
||
Critical Security Issues
|
||
|
||
1. Hardcoded InfluxDB Token (line 65)
|
||
#define INFLUX_TOKEN
|
||
"BCIZ6kcCAVgpcwSfU0PBS7m0Zb6an93kuOtizbEtlXub-uaoYp4dmIQBQYaJCv8_KE4QYYZ08jxtpaZ3TUWP-Q=="
|
||
- Risk: Credential exposure in source control
|
||
- Fix: Move to environment variable or config file
|
||
|
||
2. Unencrypted Communications
|
||
- MQTT: Plaintext on port 1883 (no TLS)
|
||
- InfluxDB: Plaintext HTTP on port 5086
|
||
- No MQTT authentication (no username/password)
|
||
|
||
3. Hardcoded IP Addresses
|
||
#define MQTT_BROKER "10.1.1.1"
|
||
#define INFLUX_HOST "10.1.1.1"
|
||
|
||
---
|
||
Code Quality Issues
|
||
|
||
┌──────────────────────┬───────────────┬──────────────────────────────────────────────┐
|
||
│ Issue │ Line │ Problem │
|
||
├──────────────────────┼───────────────┼──────────────────────────────────────────────┤
|
||
│ Memory leak │ 398-407 │ mosq_test() allocates buf but never frees it │
|
||
├──────────────────────┼───────────────┼──────────────────────────────────────────────┤
|
||
│ Missing return │ 393-407 │ mosq_test() declared int but has no return │
|
||
├──────────────────────┼───────────────┼──────────────────────────────────────────────┤
|
||
│ Buffer overflow risk │ 226, 234, etc │ sprintf() without bounds checking │
|
||
├──────────────────────┼───────────────┼──────────────────────────────────────────────┤
|
||
│ Mixed languages │ 25, 44, etc │ Polish/English comments mixed │
|
||
├──────────────────────┼───────────────┼──────────────────────────────────────────────┤
|
||
│ Dead code │ 903 │ //mosq_test() commented out │
|
||
└──────────────────────┴───────────────┴──────────────────────────────────────────────┘
|
||
|
||
---
|
||
Potential Bugs
|
||
|
||
1. Energy Spike Detection Logic (line 630)
|
||
if (prev_energy.W_Tot != 0 && (current_energy.W_Tot > prev_energy.W_Tot * 1.8 || current_energy.W_Tot <
|
||
prev_energy.W_Tot))
|
||
This rejects ANY decrease in energy (not just spikes), which may reject valid data if the counter rolls
|
||
over or resets.
|
||
|
||
2. Frequency Threshold Inconsistency
|
||
- Line 569: ±5% threshold for spike detection
|
||
- But line 641 uses 1.05/0.95 (also 5%)
|
||
- Line 688 validates 47-53 Hz (±6% from 50 Hz)
|
||
|
||
3. InfluxDB Line Protocol Syntax Issues
|
||
|
||
Line 323 (missing comma between tags and fields):
|
||
"orno,device=orno,pomiar=voltage, L1=%.2f,L2=%.2f,L3=%.2f"
|
||
Should be:
|
||
"orno,device=orno,pomiar=voltage L1=%.2f,L2=%.2f,L3=%.2f"
|
||
|
||
4. asctime() Buffer Issue (line 797)
|
||
sprintf(str_buf, "\"%s\"", asctime(timeinfo));
|
||
asctime() returns pointer to static buffer - contains newline which is then manipulated (line 798-799).
|
||
Not thread-safe.
|
||
|
||
5. Inconsistent Scaling in ORNO Registers
|
||
|
||
Line 132: L3 Volt has scale: 10 while L1/L2 have scale: 1 - potential copy-paste error?
|
||
|
||
---
|
||
Configuration (lines 26-66)
|
||
|
||
#define READ_LOOP true // true = continuous 5s polling
|
||
#define VOLTAGE_FLUCTUATION_THRESHOLD 0.05 // 5%
|
||
#define HIGH_FREQ_MODE_DURATION 300 // 300 seconds
|
||
#define ORNO_RTS_DELAY 5000 // 5ms
|
||
#define SUN2K_RTS_DELAY 5000 // 5ms
|
||
#define DELAY_BETWEEN_DEVICES 10000 // 10ms between devices
|
||
|
||
---
|
||
Modbus Register Maps
|
||
|
||
ORNO (mbReg_Orno[]) - Uses function code 3 (holding registers):
|
||
|
||
┌───────────┬─────────┬──────┬─────────────────┐
|
||
│ Register │ Address │ Type │ Scale │
|
||
├───────────┼─────────┼──────┼─────────────────┤
|
||
│ L1 Volt │ 0x000E │ U32 │ 1 │
|
||
├───────────┼─────────┼──────┼─────────────────┤
|
||
│ L2 Volt │ 0x0010 │ U32 │ 1 │
|
||
├───────────┼─────────┼──────┼─────────────────┤
|
||
│ L3 Volt │ 0x0012 │ U32 │ 10 (suspicious) │
|
||
├───────────┼─────────┼──────┼─────────────────┤
|
||
│ Tot Power │ 0x001C │ U32 │ 1 │
|
||
├───────────┼─────────┼──────┼─────────────────┤
|
||
│ ... │ ... │ ... │ ... │
|
||
└───────────┴─────────┴──────┴─────────────────┘
|
||
|
||
SUN2000 (mbReg[]) - Various data types:
|
||
- rgData - timestamps (lines 107-109)
|
||
- rgI32 - signed 32-bit (power values)
|
||
- rgU32 - unsigned 32-bit (energy counters)
|
||
- rgI16 - signed 16-bit (temperature)
|
||
- rgU16 - unsigned 16-bit (status codes)
|
||
|
||
---
|
||
Recommendations
|
||
|
||
1. Security: Move credentials to environment variables
|
||
2. Add TLS: Configure MQTT and InfluxDB with TLS certificates
|
||
3. Fix InfluxDB format: Remove comma between tags/fields in line protocol
|
||
4. Buffer safety: Replace sprintf() with snprintf()
|
||
5. Memory: Fix leak in mosq_test() or remove unused function
|
||
6. Scaling: Verify L3 Volt scale (10 vs 1) for ORNO
|
||
7. Energy logic: Consider rollover handling for energy counters
|
||
8. Logging: Add structured logging instead of printf
|
||
|
||
---
|
||
Summary
|
||
|
||
This is functional embedded systems code for energy monitoring with reasonable error handling and data
|
||
validation. The main concerns are security (hardcoded credentials) and potential data quality issues
|
||
(InfluxDB line protocol format, scaling inconsistencies). The spike detection and voltage fluctuation
|
||
features show thoughtful defensive programming.
|
||
|
||
✻ Sautéed for 52s
|
||
|
||
--------------------------------------------------------
|
||
|
||
|
||
❯ /model
|
||
⎿ Set model to opus (claude-opus-4-6)
|
||
|
||
❯ analyse modb_orno3.c
|
||
⎿ Not logged in · Please run /login
|
||
|
||
❯ run /login
|
||
⎿ Not logged in · Please run /login
|
||
|
||
❯ run /login
|
||
⎿ Not logged in · Please run /login
|
||
|
||
❯ /login
|
||
⎿ Login successful
|
||
|
||
❯ analyse modb_orno3.c
|
||
|
||
● Searched for 1 pattern, read 1 file (ctrl+o to expand)
|
||
|
||
● Analiza modb_orno3.c
|
||
|
||
Opis ogólny
|
||
|
||
Program w C do odczytu danych z dwóch urządzeń Modbus RTU przez port szeregowy:
|
||
- ORNO (slave 2) — trójfazowy licznik energii
|
||
- SUN2000 (slave 3) — falownik Huawei SUN2000
|
||
|
||
Odczytane dane są wysyłane do MQTT (broker 10.1.1.1:1883) i InfluxDB v2 (10.1.1.1:5086).
|
||
|
||
---
|
||
Architektura
|
||
|
||
┌──────────────┬──────────────────────────────────────────────────────┐
|
||
│ Warstwa │ Opis │
|
||
├──────────────┼──────────────────────────────────────────────────────┤
|
||
│ Modbus RTU │ libmodbus, port /dev/ttyUSB0, 9600 baud │
|
||
├──────────────┼──────────────────────────────────────────────────────┤
|
||
│ MQTT │ libmosquitto, topic /energy/orno/* i /energy/SUN2K │
|
||
├──────────────┼──────────────────────────────────────────────────────┤
|
||
│ InfluxDB │ Surowe HTTP POST na /api/v2/write, line protocol │
|
||
├──────────────┼──────────────────────────────────────────────────────┤
|
||
│ Pętla główna │ do/while co 5s (normalnie) lub 10ms (tryb high-freq) │
|
||
└──────────────┴──────────────────────────────────────────────────────┘
|
||
|
||
---
|
||
Znalezione problemy i uwagi
|
||
|
||
1. Wyciek tokenu InfluxDB (linia 65)
|
||
|
||
Token jest zahardkodowany w kodzie źródłowym. Powinien być w zmiennej środowiskowej lub pliku
|
||
konfiguracyjnym.
|
||
|
||
2. Bug: modbus_set_slave przed sprawdzeniem ctx (linia 459-460)
|
||
|
||
modbus_set_slave(ctx, ORNO_SLAVE); // linia 459 - ctx może być NULL
|
||
if (!ctx) { ... } // linia 460 - sprawdzenie za późno
|
||
modbus_set_slave zostanie wywołane na NULL pointer zanim nastąpi walidacja. Crash gwarantowany jeśli
|
||
modbus_new_rtu zawiedzie.
|
||
|
||
3. Bug w InfluxDB line protocol - nadmiarowy przecinek (linia 323)
|
||
|
||
"orno,device=orno,pomiar=voltage, L1=%.2f,L2=%.2f,L3=%.2f"
|
||
// ^ spacja+przecinek zamiast spacji
|
||
Powinno być pomiar=voltage L1=... (spacja oddziela tagi od pól). Dodatkowy przecinek po voltage powoduje
|
||
błąd parsowania w InfluxDB — dane napięcia prawdopodobnie nie są zapisywane.
|
||
|
||
4. Tryb high-frequency polling — 10ms bez throttlingu (linia 897)
|
||
|
||
Przy wykryciu fluktuacji napięcia program przełącza się na usleep(10000) (10ms) na 300 sekund. W tym
|
||
trybie:
|
||
- Program robi ~100 odczytów/s po Modbus RTU (9600 baud to ~1 odczyt/s realnie)
|
||
- Każdy odczyt otwiera nowe połączenie TCP do InfluxDB → tysiące krótkich połączeń
|
||
- Timeout Modbus to 0.9s — więc realna częstotliwość i tak będzie ~1/s, ale logika jest myląca
|
||
|
||
5. Brak modbus_set_slave po stronie SUN2K idzie przed ctx check (linia 720 vs 714)
|
||
|
||
Tutaj jest OK — modbus_set_slave jest po sprawdzeniu ctx. Ale dla ORNO (pkt 2) jest bug.
|
||
|
||
6. Połączenie TCP do InfluxDB tworzone na każdy pomiar (linia 264-318)
|
||
|
||
Każde wywołanie influx_send_post otwiera nowy socket, wysyła dane i zamyka. W normalnym trybie to ~6
|
||
połączeń co 5s (OK), ale w trybie high-freq to setki na minutę. Lepiej: batch writes lub persistent
|
||
connection.
|
||
|
||
7. Brak odczytu odpowiedzi z InfluxDB (linia 317)
|
||
|
||
Po wysłaniu danych socket jest zamykany bez czytania odpowiedzi HTTP. Nie wiadomo czy zapis się powiódł
|
||
(np. 401 Unauthorized, 400 Bad Request).
|
||
|
||
8. sprintf bez ograniczenia bufora (linie 227, 235, 243, 251, 259, 401)
|
||
|
||
Używane sprintf zamiast snprintf — potencjalny buffer overflow przy nieoczekiwanych danych float (np.
|
||
bardzo duże wartości).
|
||
|
||
9. Spike detection nie działa dla ujemnych mocy (linia 619)
|
||
|
||
current_power.P_Tot > prev_power.P_Tot * 1.8 || current_power.P_Tot < prev_power.P_Tot * 0.2
|
||
Jeśli moc jest ujemna (oddawanie do sieci), mnożenie przez 1.8 daje bardziej ujemną wartość, a 0.2 mniej
|
||
ujemną — logika się odwraca.
|
||
|
||
10. Energy spike — asymetryczna walidacja (linia 630)
|
||
|
||
current_energy.W_Tot > prev_energy.W_Tot * 1.8 || current_energy.W_Tot < prev_energy.W_Tot
|
||
Górny próg to 180%, ale dolny to dokładnie prev (jakikolwiek spadek = spike). Energia powinna być
|
||
monotoniczne rosnąca, więc logika dolnego progu jest OK, ale wartość 1.8 dla górnego może być za wysoka.
|
||
|
||
11. mosq_test() — memory leak (linia 398)
|
||
|
||
malloc(64) bez free. Drobnostka bo funkcja nie jest używana (zakomentowana w main).
|
||
|
||
12. Różne parametry seryjne ORNO vs SUN2K
|
||
|
||
- ORNO: 9600, Even parity, 8, 1 (linia 457)
|
||
- SUN2K: 9600, No parity, 8, 1 (linia 713)
|
||
|
||
To jest poprawne (Huawei SUN2000 domyślnie używa N-8-1, ORNO domyślnie E-8-1), ale warto mieć świadomość
|
||
że oba urządzenia mają różne konfiguracje.
|
||
|
||
13. Niestandardowy include (linia 4)
|
||
|
||
#include </usr/include/modbus/modbus-rtu.h>
|
||
Powinno być #include <modbus/modbus-rtu.h> — absolutna ścieżka uniemożliwia przenośność.
|
||
|
||
---
|
||
Podsumowanie priorytetów napraw
|
||
|
||
┌───────────┬────────────────────────────────────────────────────┐
|
||
│ Priorytet │ Problem │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Krytyczny │ Bug NULL pointer dereference ORNO (pkt 2) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Krytyczny │ InfluxDB line protocol — zły format napięć (pkt 3) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Wysoki │ Token InfluxDB w kodzie (pkt 1) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Wysoki │ Spike detection dla ujemnych mocy (pkt 9) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Średni │ Brak odczytu odpowiedzi InfluxDB (pkt 7) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Średni │ sprintf → snprintf (pkt 8) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Niski │ Batching InfluxDB writes (pkt 6) │
|
||
├───────────┼────────────────────────────────────────────────────┤
|
||
│ Niski │ Niestandardowy include path (pkt 13) │
|
||
└───────────┴────────────────────────────────────────────────────┘
|
||
|
||
Chcesz żebym naprawił którekolwiek z tych problemów?
|
||
|
||
✻ Cooked for 52s
|
||
|
||
──────────────────────────────────────────────────────────────────────────────────────────────────────────
|
||
❯ napraw krytyczne problemy (pkt 2 i 3
|
||
|
||
──────────────────────────────────────────────────────────────────────────────────────────────────────
|
||
? for shortcuts
|
||
|
||
|