重构大疆电机和达妙电机的控制数据合并方法,修复掉线检测失效问题#55
Conversation
There was a problem hiding this comment.
Pull request overview
该 PR 重构了大疆 GM6020 与达妙一控四(DMmulti)电机的 CAN 控制数据合并逻辑,目标是让合并过程能够同时触发双方的掉线检测,从而修复“掉线检测失效”的问题。
Changes:
- 将 GM6020 / DMmulti 的控制数据合并由
operator+改为getMergedControlData(...),返回合并后的 8 字节控制帧指针 - 合并时改为通过
getMotorControlData()获取双方控制帧,以触发双方的连接状态检测 - 为两类电机新增
m_mergedData[8]作为合并输出缓冲区
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| GSRL/Device/src/dvc_motor.cpp | 实现新的 getMergedControlData 合并逻辑,并通过 getMotorControlData() 触发双方掉线检测 |
| GSRL/Device/inc/dvc_motor.hpp | 更新接口声明并新增 m_mergedData 成员作为合并输出缓冲区 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @return const uint8_t* 合并后的8字节CAN控制数据 | ||
| * @note 返回的指针指向内部静态缓冲区, 下次调用会覆盖 |
There was a problem hiding this comment.
注释里写“返回的指针指向内部静态缓冲区”,但这里返回的是对象成员 m_mergedData(每个对象一份),并非 static 缓冲区。建议将说明改为“内部成员缓冲区/对象内缓冲区”,并明确仅同一对象的下次调用会覆盖。
| * @return const uint8_t* 合并后的8字节CAN控制数据 | |
| * @note 返回的指针指向内部静态缓冲区, 下次调用会覆盖 | |
| * @return const uint8_t* 指向合并后的8字节CAN控制数据的指针 | |
| * @note 返回的指针指向当前对象的内部成员缓冲区m_mergedData, 仅当前对象的后续调用会覆盖该缓冲区内容 |
| const uint8_t *MotorGM6020::getMergedControlData(MotorGM6020 &otherMotor) | ||
| { | ||
| const uint8_t *selfData = this->getMotorControlData(); | ||
| const uint8_t *otherData = otherMotor.getMotorControlData(); | ||
|
|
||
| if (m_motorControlMessageID != otherMotor.m_motorControlMessageID) { | ||
| return *this; | ||
| return selfData; | ||
| } | ||
|
|
||
| MotorGM6020 combineMotor = *this; | ||
| if (otherMotor.m_djiMotorID < 5) { | ||
| uint8_t offset = otherMotor.m_djiMotorID * 2 - 2; | ||
| combineMotor.m_motorControlData[offset] = otherMotor.m_motorControlData[offset]; | ||
| combineMotor.m_motorControlData[offset + 1] = otherMotor.m_motorControlData[offset + 1]; | ||
| } else { | ||
| uint8_t offset = otherMotor.m_djiMotorID * 2 - 10; | ||
| combineMotor.m_motorControlData[offset] = otherMotor.m_motorControlData[offset]; | ||
| combineMotor.m_motorControlData[offset + 1] = otherMotor.m_motorControlData[offset + 1]; | ||
| } | ||
| return combineMotor; | ||
| memcpy(m_mergedData, selfData, 8); | ||
| uint8_t offset = (uint8_t)(((otherMotor.m_djiMotorID - 1) & 0x3) * 2); | ||
| m_mergedData[offset] = otherData[offset]; | ||
| m_mergedData[offset + 1] = otherData[offset + 1]; | ||
| return m_mergedData; |
There was a problem hiding this comment.
当前实现每次都用 this->getMotorControlData() 作为合并的基底并 memcpy 覆盖整帧,因此连续多次调用该方法并不能像之前的 operator+ 那样累积合并 3+ 个电机的数据(会反复从“this 的原始控制帧”重新开始)。如果一条控制帧需要承载多电机(如一控四),建议提供可累积的合并接口(例如传入/返回可继续合并的缓冲区,或提供合并 N 个电机的函数),否则上层很难正确拼出完整 8 字节控制帧。
| memcpy(m_mergedData, selfData, 8); | ||
| uint8_t offset = (uint8_t)(((otherMotor.m_djiMotorID - 1) & 0x3) * 2); | ||
| m_mergedData[offset] = otherData[offset]; |
There was a problem hiding this comment.
这里新增使用 memcpy,但本文件并未直接包含 <cstring>/<string.h>,当前是通过 alg_general.hpp 的传递包含拿到声明,耦合较强。建议在本文件或对应头文件中显式包含 <cstring>(或 <string.h>)以避免后续移除间接 include 导致编译问题。
| * @brief 合并两个同控制ID的一控四固件达妙电机CAN控制数据, 同时触发双方掉线检测 | ||
| * @param otherMotor 另一个同控制ID的达妙一控四电机 | ||
| * @return const uint8_t* 合并后的8字节CAN控制数据 | ||
| * @note 返回的指针指向内部静态缓冲区, 下次调用会覆盖 |
There was a problem hiding this comment.
注释里写“返回的指针指向内部静态缓冲区”,但这里返回的是对象成员 m_mergedData(每个对象一份),并非 static 缓冲区。建议将说明改为“内部成员缓冲区/对象内缓冲区”,并明确仅同一对象的下次调用会覆盖。
| * @note 返回的指针指向内部静态缓冲区, 下次调用会覆盖 | |
| * @note 返回的指针指向当前对象的内部成员缓冲区, 仅当前对象的下次调用会覆盖 |
| const uint8_t *MotorDMmulti::getMergedControlData(MotorDMmulti &otherMotor) | ||
| { | ||
| const uint8_t *selfData = this->getMotorControlData(); | ||
| const uint8_t *otherData = otherMotor.getMotorControlData(); | ||
|
|
||
| if (m_motorControlMessageID != otherMotor.m_motorControlMessageID) { | ||
| return *this; | ||
| return selfData; | ||
| } | ||
|
|
||
| MotorDMmulti combineMotor = *this; | ||
| uint8_t offset = (uint8_t)(((otherMotor.m_dmMotorID - 1) & 0x3) * 2); | ||
| combineMotor.m_motorControlData[offset] = otherMotor.m_motorControlData[offset]; | ||
| combineMotor.m_motorControlData[offset + 1] = otherMotor.m_motorControlData[offset + 1]; | ||
| return combineMotor; | ||
| memcpy(m_mergedData, selfData, 8); | ||
| uint8_t offset = (uint8_t)(((otherMotor.m_dmMotorID - 1) & 0x3) * 2); | ||
| m_mergedData[offset] = otherData[offset]; | ||
| m_mergedData[offset + 1] = otherData[offset + 1]; | ||
| return m_mergedData; |
There was a problem hiding this comment.
与上面的 GM6020 同理:该实现每次都从 this->getMotorControlData() 重新拷贝整帧,再覆盖另一个电机的 2 字节,因此无法通过多次调用累积合并 3~4 个电机(会丢失此前已合并的电机数据)。考虑到该类语义是一控四,建议提供可累积/可合并多个电机的接口,或至少在注释中明确该方法仅合并两台电机且不支持链式累积。
| memcpy(m_mergedData, selfData, 8); | ||
| uint8_t offset = (uint8_t)(((otherMotor.m_dmMotorID - 1) & 0x3) * 2); | ||
| m_mergedData[offset] = otherData[offset]; |
There was a problem hiding this comment.
同样地,这里新增使用 memcpy,建议显式包含 <cstring>/<string.h>,避免依赖其他头文件的传递包含导致后续编译脆弱。
No description provided.