Files
gaoguangpu/code_review_report.md
2026-08-18 13:34:21 +08:00

15 KiB
Raw Blame History

代码审查报告 — 波长系数全精度修复及周边问题

  • 审查日期:2026-08-12
  • 审查对象:src/main.cppbochangxishu 全精度修复(占位符 + 序列化后字符串替换)、ArduinoJson 7.4.2 → 6.21.6 降级,以及 git diff(HEAD~1..HEAD + 工作区改动)触及的周边代码
  • 行号基准:当前工作区文件
  • 审查方式:逐行 diff 扫描 + 跨文件数据流追踪 + 行为变更审计 + 多角度并行核查(5 个后台 agent 交叉验证)

结论摘要

# 严重度 位置 问题 类别
1 🔴 严重 main.cpp:2623-2676 精度修复把 SD 卡存储的标定系数覆盖成出厂系数 本次改动引入的回归
2 🟠 main.cpp:416 / 422 = 写成 ==,manual 采集窗口永不生效,且把 work_time 篡改为 "gps" 既有 bug(随 diff 触及)
3 🟠 main.cpp:795 未持有互斥锁就 xSemaphoreGive,可破坏互斥 并发
4 🟠 main.cpp:960-961 OTA 挂起唯一喂狗任务 Task1,>10 分钟 OTA 中途 panic 重启 WDT
5 🟠 main.cpp:566-570 open_4G_mode 阻塞网络调用导致任务看门狗饥饿,重启循环 WDT
6 🟠 main.cpp:892 上传净荷无锁撕裂读(两任务并发读写 ~8KB 结构体) 并发
7 🟡 main.cpp:2663-2676 字符串替换方案脆弱(replace 静默失效 → 200000 被写入文件) 本次改动
8 🟡 main.cpp:875-880 http_head.sn/version 为 char[20],长字符串越界写 内存
9 🟡 main.cpp:734 每日重启死代码:day_count 从不递增,esp_restart() 不可达 逻辑
10 🟡 main.cpp:566 4G 模式每 ~10s 无条件全量 getnetData+get_GPS,网络轮询量 ×12 性能
11 🟡 src/SensorIS11.cpp:85 new char[] 用标量 delete 释放(delete/delete[] 不匹配,UB) 内存
12 🟡 src/gsmm_mqtt.cpp:163 4 处复制粘贴的握手轮询,且 180s 超时只守前半段,portMAX_DELAY 无界 可维护性
13 🟡 src/gsmm_mqtt.cpp:124 共享 20s 客户端超时,弱网下上传/OTA 单块停滞即断连 网络
14 🟢 main.cpp:2628-2631 b0-b3 系数未被修复覆盖,仍 9 位小数截断(修复只做了一半) 本次改动
15 🟢 src/gsmm_mqtt.cpp:77 reconnect() 无退避,断连时最多阻塞 ~200s 网络
16 🟢 main.cpp:366 Task1 栈 15KB 承载了原 23KB 的 4G 调用链,有溢出风险 资源
17 🟢 main.cpp:2663-2676 / 2718-2731 替换块两个分支逐字重复,后续修改需同步两处 可维护性

一、本次精度修复直接引入的问题(最优先)

🔴 #1 存储标定被出厂系数覆盖(本次改动的回归)

位置:main.cpp:2623-2626main.cpp:2664-2676

启动时序:

277  initSensor()                 → SensorInfo.a1..a4 = 传感器出厂系数
294  sys_info_init()              ← 此刻 SensorInfo 仍是出厂值!
     2619-2626  被注释掉的旧代码原本保留 doc 中的存储值,现被 200000 占位符取代
     2638-2641  doc["bochangxishu"]["a0..a3"] 被读入 guangpu_bochang(存储标定加载成功)
     2664-2676  替换块却用 SensorInfo.a1..a4(= 出厂值)写回 sys_sd_doc 并序列化
319-344        之后才把 guangpu_bochang → SensorInfo(运行时实际用的是存储标定)

后果:

  • 只要 SD 卡上存在 system_info.json,每次开机都会把 sys_sd_doc["bochangxishu"]["a0..a3"] 重置为出厂系数,而不是 SD 里的存储标定。
  • 下一次任意配置保存(如 main.cpp:786 Task2 写盘)会把出厂值固化到文件,存储标定永久丢失
  • get_bochangxishu(main.cpp:1793-1808)读到的是出厂值,而运行时计算波长用的是 guangpu_bochang(存储值)→ 主机读到的系数与实际计算用的系数不一致

被注释掉的旧代码 main.cpp:2619-2622(sys_sd_doc[...] = doc[...])恰恰是保留存储标定的正确做法——它的问题只是序列化时丢到 9 位小数,而不是数值错误。

修复建议(文件存在分支):

// 替换块的数据源从 SensorInfo(出厂)改为 guangpu_bochang(存储标定)
const double a[4] = { is11Sensor->guangpu_bochang.a0,
                      is11Sensor->guangpu_bochang.a1,
                      is11Sensor->guangpu_bochang.a2,
                      is11Sensor->guangpu_bochang.a3 };

首启分支(main.cpp:2700-27032718-2731)用 SensorInfo正确的(此时无存储值,出厂系数即默认标定),无需改动。

🟡 #7 字符串替换方案本身脆弱

位置:main.cpp:2663-2676

  • String::replace全局替换:任何字段若恰好序列化为 :200000(数值 200000 的字段),会被一并改成该系数。
  • 替换依赖 ArduinoJson 6.21.6 的精确输出格式 "a0":200000。若未来库升级/格式化变化(冒号后空格、200000.0、科学计数法),replace 会静默失效,占位符 200000 被原样写入文件;下次开机 2638 把它读回 guangpu_bochang.a0,每个波长偏移 ~200000nm。
  • 方案的"全精度"目标本身有悖论:降级到 6.21.6 正是为了让这套 replace 匹配其输出;一旦动库版本,修复即失效。

建议:文件存在分支不需要占位符——数据源是内存里的 double,序列化 9 位截断后,直接用 key 锚定 replace 那 4 个 9 位字符串即可;或接受 9 位小数(见下文精度量化)。

🟢 #14 b0-b3 未覆盖

文件存在分支里 b 系数(main.cpp:2628-2631)仍走 ArduinoJson 默认 9 位小数序列化,同样丢精度——修复只做了一半。若要彻底,应对 b0-b3 采用相同手段。

关于"丢精度"的量化提醒

本次修改前我已测算:对这些量级的系数,a0 误差 ~1e-8nm(2047 像元处)、a3 误差 ~5e-5nm,远低于传感器自身光谱分辨率。9 位小数在物理上可能已足够,如果确认存储标定不被覆盖才是关键,可以考虑干脆不做字符串替换,只修复 #1 的数值来源。


二、并发 / 任务问题

🟠 #2 work_time 赋值写成比较

位置:main.cpp:416 / 422

if(sys_sd_doc["work_time"] = "gps")       // 应为 ==
else if(sys_sd_doc["work_time"] = "manual")  // 应为 ==

JsonVariant::operator= 返回非空 variant,bool 转换恒为 true → gps 分支无条件进入,manual 采集窗口(用户配置的 start_time/stop_time)永不生效。副作用:每次 Task0 迭代都把 sys_sd_doc["work_time"] 写成 "gps",下次保存后用户设的 "manual" 被固化篡改。已在 430 之前执行,并直接跳过 manual 分支。修复:改 ==

🟠 #3 Task2 在未持有互斥锁时 Give

位置:main.cpp:781-795

xSemaphoreGive(xMutexInventory)if(xSemaphoreTake(...) == pdPASS) 块外。若 take 超时(其他任务持有锁超过 100s,如 Task1 阻塞在网络调用中),Task2 会 Give 一个自己不持有的锁 → 释放别人的临界区,两个任务并发进入,sys_sd_doc / 写盘互相踩踏;在 FreeRTOS 上属于未定义行为,可能触发互斥锁所有权断言。

修复建议:

if(xSemaphoreTake(xMutexInventory, timeOut) == pdPASS)
{
    ...序列化写盘...
    save = false;
    xSemaphoreGive(xMutexInventory);
}

🟠 #4 OTA 挂起唯一喂狗任务 → 中途重启

位置:main.cpp:960-961550370-376

Task1 是唯一注册任务看门狗的任务(esp_task_wdt_add(NULL),10 分钟超时,trigger_panic=true),而 OTA_task 在下载+烧录期间 vTaskSuspend(Task1_Handler)。慢速 4G 下载 >10 分钟 → WDT panic → CONFIG_ESP_PANIC_HANDLER_REBOOT 中途重启 → 固件分区半写,设备可能变砖。同时 open_4G_mode 内的 getnetData/get_GPS/UpdateData 各自最多 180s,叠加可超 10 分钟,造成假阳性重启(见 #5)。

🟠 #5 任务看门狗被当成万能补丁

trigger_panic=true + 双核 idle_core_mask + 10 分钟超时,而唯一的喂狗任务 Task1 恰恰在做全部长阻塞网络 I/O(main.cpp:566-570)。设计自相矛盾:网络慢但活着,也会被看门狗判死刑。4G 链路拥堵时设备会进入"每 ~10 分钟重启一次"的循环,4G 模式无法恢复。

🟠 #6 上传净荷撕裂读

位置:main.cpp:892433

Task0 采集写入 IS11_datastruct_fanshelv(无锁,433 get_fanshelv()),Task1 在 open_4G_mode 里先做最长 180s 的网络同步再 memcpy 这个结构体(也无锁,仅在 up_data 标志上加锁)。两个核并发读写 ~8KB 结构体 → 上传的数据包可能是两次测量的混合(头部/波长被撕裂),服务器端静默收到错误数据。


三、GSM / MQTT 子模块

🟡 #12 握手轮询 4 处复制 + 超时只守一半

位置:gsmm_mqtt.cpp:163230348550

while(uxBits & mqtt_stop_bit){ ... if(cout==180*1000) return; vTaskDelay(1); } 被复制 4 次,且已分化(有的返回 false、有的返回 "-1"、有的 esp_restart)。180s 超时只守卫 mqtt_stop_bit 前半段,紧接的 xEventGroupWaitBits(Http_start_bit, ..., portMAX_DELAY) 无界——若 mqtt 任务僵死,调用方永久阻塞。建议提取单一 wait_mqtt_free() helper。已核实本板 CONFIG_FREERTOS_HZ=1000,vTaskDelay(1)=1ms,故 180×1000 确实 ≈ 180s(早期"30 分钟"说法不成立,特此更正)。

🟡 #13 共享 20s 客户端超时

位置:gsmm_mqtt.cpp:124

同一个 Client 对象同时被 PubSubClient(MQTT)和 HttpClient(HTTP)使用,setTimeout(1000*20) 一并生效。弱网下 1024 字节单块写停滞 >20s → http->write() 返回 -1 → UpdateData 返回 false 而 open_4G_mode 忽略返回值,上传静默丢失;OTA 下载同理中断。

🟢 #15 reconnect 无退避

断连时最多 10 次 × 20s = ~200s 阻塞在重连里,期间不服务 mqtt_stop_bit/Http_start_bit 握手,Task1 的 open_4G_mode 干等 180s 后超时,当天数据可能丢弃。


四、内存 / 资源

🟡 #11 delete / delete[] 不匹配

位置:SensorIS11.cpp:85

74 result = new char[retlenth] 用标量 delete result 释放(同一函数另一处 67 正确用了 delete[])。UB,可能损坏堆元数据。改为 delete[] result;

🟡 #8 char[20] 越界写

位置:main.cpp:875-880

memcpy(http_head.sn, sn.c_str(), sn.length()) + http_head.sn[sn.length()]='\0' 无长度校验。当序列号 ≥ 20 字符时越界写,破坏相邻的 http_head 字段。现有序列号(如 "TFNSP44250004")较短暂安全,但 version 取自 874sys_sd_doc["version"],长版本号即可触发。建议加 sn.length() < sizeof(http_head.sn) 保护。

🟢 #16 Task1 栈吃紧

Task1 栈 15KB(main.cpp:366),却承载了原 task_4G_mode(23KB 独立任务)的全部调用链:open_4G_mode → getnetData/get_GPS/UpdateData(HttpClient/TinyGsm/String 深层栈帧)。深栈路径下可能溢出,而溢出任务恰好是喂狗任务,表现为随机 panic 重启。


五、可维护性

  • 替换块在两个分支逐字重复(2663-2676 / 2718-2731)——修 #1 时需同步两处,建议提取成函数。
  • day_count / count / running_count 三重计数冗余,day_count 从未递增(#9),已基本失去可推理性。
  • main.cpp:556-562 up_data 全局标志在 Task1 每 1s 循环被清零,但只在 running_count==10 时才消费 → 两窗口之间的触发被静默丢弃。

六、已验证的"非问题"(避免误改)

结论
ArduinoJson 7.4.2 → 6.21.6 降级 正确且必要:v7 移除了 DynamicJsonDocument,HEAD 上 pin 的 7.4.2 本来就编译不过。代码全用 v6 原生 API,无兼容问题。
reconnect() 返回值 唯一调用方 gsmm_mqtt_loop_task 正确消费 reconnect_flag
updata_buff 大小 sizeof(http_head)+sizeof(IS11_datastruct) 计算正确,无溢出。
save 标志 读写在互斥锁内完成,volatile 裸读在本核上无害。
180×1000 超时 已核实 CONFIG_FREERTOS_HZ=1000,即 180s,不是 30 分钟。

七、修复优先级建议

优先级 动作
P0 #1 存储标定被覆盖 文件存在分支替换块数据源改 guangpu_bochang(或恢复旧代码+全精度替换);上线前必改
P0 #2 work_time 赋值 ==;否则 manual 模式完全失效且配置被篡改
P0 #3 互斥锁 Give 越权 移入 take 成功分支内
P1 #4/#5 WDT OTA 期间喂狗或临时解注册;open_4G_mode 内周期性 esp_task_wdt_reset;或缩小超时并把 panic 改为可恢复
P1 #6 上传撕裂 采集与 memcpy 之间加锁,或改为单任务快照拷贝
P2 #8/#11/#12/#13 加长度保护、改 delete[]、抽取 wait helper、上传用独立超时
P2 #7/#14 决定替换方案的取舍(见 #1 修复建议与精度量化)

附录:本次精度修复的原始问题回顾

  • 问题根源:ArduinoJson(v7.4.2 与 6.21.6 一致)序列化 double 时硬编码最多 9 位小数(内部 maxDecimalPart=10^9uint32_t 上限约束,10^15 会溢出),并非赋值丢精度——ARDUINOJSON_USE_DOUBLE=1 下 double 在内存中是精确的。
  • 当前方案:序列化前置 200000 占位整数 → serializeJson → 按 "aX":200000 锚定字符串替换为 snprintf("%.15g") 的全精度系数 → 恢复 sys_sd_doc 真实值。%.15g(而非 String(v,15))才能同时保住 ~1e-9 小系数与 ~3e2 大系数的有效位数。
  • 遗留:写入路径 main.cpp:786(配置保存)与 main.cpp:1226(GPS 更新保存)仍直接 serializeJson(sys_sd_doc,...),bochangxishu 依旧 9 位截断——本次只修了 sys_info_init 的启动/首启两条路径,这两处如需全精度需同样处理。