Skip to content

feat(i2c): Add end-to-end I2C support for RmcsBoard and CBoard #43

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

Closed
gqsdjhh wants to merge 11 commits into from
Closed

feat(i2c): Add end-to-end I2C support for RmcsBoard and CBoard #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
df5cc1b
feat(i2c): Add I2C0 protocol and RmcsBoard support
Apr 7, 2026
44ff397
feat(c_board): Add I2C2 transport support
Apr 8, 2026
12bce2b
fix(c_board): Replace queued I2C with blocking transfers
Apr 9, 2026
20ea882
fix(c_board): Recover i2c dma transfers on timeout
gqsdjhh Apr 11, 2026
9e7d481
refactor(c_board): Clarify logical i2c0 to i2c2 mapping
gqsdjhh Apr 11, 2026
c561efe
fix(c_board): Lower i2c irq priority to avoid usb conflicts
gqsdjhh Apr 11, 2026
13b60a8
fix(rmcs_board): Update hpm_sdk to restore hpm5300evk board definitions
gqsdjhh Apr 11, 2026
2b2895d
fix(i2c): Harden Error Handling And Preserve Uplink Responses
gqsdjhh Apr 12, 2026
15a6b43
fix(i2c): Preserve Error Context And Harden Invalid Request Handling
gqsdjhh Apr 14, 2026
31f06e3
fix(protocol): Reject Empty I2C Transfers
gqsdjhh Apr 14, 2026
f04f753
Merge remote-tracking branch 'upstream/main' into i2c
gqsdjhh Apr 14, 2026
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
Empty file added .codex
View file
Open in desktop
Empty file.
32 changes: 32 additions & 0 deletions core/include/librmcs/data/datas.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ enum class DataId : uint8_t {
kUart3 = 14,

kImu = 15,

kI2c0 = 16,
};

struct CanDataView {
Expand Down Expand Up @@ -79,6 +81,28 @@ struct GyroscopeDataView {
int16_t z;
};

struct I2cDataView {
uint8_t slave_address;
std::span<const std::byte> payload;
bool has_register = false;
uint8_t reg_address = 0;
};

struct I2cReadConfigView {
uint8_t slave_address;
uint16_t read_length = 0;
bool has_register = false;
uint8_t reg_address = 0;
};

struct I2cErrorView {
uint8_t slave_address;
uint16_t data_length = 0;
bool has_register = false;
uint8_t reg_address = 0;
bool is_read = false;
};

class DataCallback {
public:
DataCallback() = default;
Expand All @@ -100,6 +124,14 @@ class DataCallback {
virtual void accelerometer_receive_callback(const AccelerometerDataView& data) = 0;

virtual void gyroscope_receive_callback(const GyroscopeDataView& data) = 0;

virtual bool i2c_receive_callback(DataId id, const I2cDataView& data) = 0;

virtual void i2c_error_callback(DataId id, const I2cErrorView& data) {
i2c_error_callback(id, data.slave_address);
}

virtual void i2c_error_callback(DataId id, uint8_t slave_address) = 0;
Comment on lines +127 to +134
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

i2c_error_callback 现在把新增错误上下文全丢掉了。

协议层和反序列化层已经有 I2cErrorView，其中包含 is_read、reg_address 和 data_length，但这里仍只把 slave_address 往上抛。这样从 host/src/protocol/handler.cpp:98-107 到 host/include/librmcs/agent/c_board.hpp:151-160 / host/include/librmcs/agent/rmcs_board.hpp:151-163 这条链路都会丢失新增信息，SDK 使用者拿不到 PR 里补充的错误上下文。建议把回调签名改成传 const I2cErrorView&。

建议修改 
-    virtual void i2c_error_callback(DataId id, uint8_t slave_address) = 0;
+    virtual void i2c_error_callback(DataId id, const I2cErrorView& data) = 0;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@core/include/librmcs/data/datas.hpp` around lines 127 - 130, The
i2c_error_callback currently only forwards slave_address and drops new error
context; change its signature in Data:: (datas.hpp) from virtual void
i2c_error_callback(DataId id, uint8_t slave_address) to virtual void
i2c_error_callback(DataId id, const I2cErrorView& error) and propagate that type
through the call chain (update protocol handler code that invokes
i2c_error_callback in host/src/protocol/handler.cpp to pass an I2cErrorView, and
update the receiving declarations in host/include/librmcs/agent/c_board.hpp and
host/include/librmcs/agent/rmcs_board.hpp to match), ensuring callers use the
fields (is_read, reg_address, data_length, slave_address) from I2cErrorView
instead of the lone uint8_t.

};

} // namespace librmcs::data
106 changes: 106 additions & 0 deletions core/src/protocol/deserializer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ coroutine::LifoTask<void> Deserializer::process_stream() {
case FieldId::kUart3: success = co_await process_uart_field(id); break;
case FieldId::kGpio: success = co_await process_gpio_field(id); break;
case FieldId::kImu: success = co_await process_imu_field(id); break;
case FieldId::kI2c0: success = co_await process_i2c_field(id); break;
default: break;
}
if (!success)
Expand Down Expand Up @@ -289,4 +290,109 @@ coroutine::LifoTask<bool> Deserializer::process_imu_field(FieldId) {
co_return true;
}

coroutine::LifoTask<bool> Deserializer::process_i2c_field(FieldId field_id) {
I2cHeader::PayloadEnum payload_type;
uint8_t slave_address = 0;
uint16_t data_length = 0;
bool has_register = false;
bool error_flag = false;
{
const auto* header_bytes = co_await peek_bytes(sizeof(I2cHeader));
if (!header_bytes) [[unlikely]]
co_return false;

auto header = I2cHeader::CRef{header_bytes};
payload_type = header.get<I2cHeader::PayloadType>();
has_register = header.get<I2cHeader::HasRegister>();
error_flag = header.get<I2cHeader::ErrorFlag>();
slave_address = header.get<I2cHeader::SlaveAddress>();
data_length = header.get<I2cHeader::DataLength>();
consume_peeked();
}

if (slave_address > 0x7F) [[unlikely]]
co_return false;

switch (payload_type) {
case I2cHeader::PayloadEnum::kWrite:
case I2cHeader::PayloadEnum::kReadResult: {
if (error_flag) [[unlikely]]
co_return false;
if (payload_type == I2cHeader::PayloadEnum::kWrite && data_length == 0) [[unlikely]]
co_return false;

data::I2cDataView data_view{};
data_view.slave_address = slave_address;
data_view.has_register = has_register;

if (has_register) {
const auto* register_bytes = co_await peek_bytes(1);
if (!register_bytes) [[unlikely]]
co_return false;
data_view.reg_address = std::to_integer<uint8_t>(register_bytes[0]);
consume_peeked();
}

if (data_length) {
const auto* payload_bytes = co_await peek_bytes(data_length);
if (!payload_bytes) [[unlikely]]
co_return false;
data_view.payload = std::span<const std::byte>{payload_bytes, data_length};
consume_peeked();
} else {
data_view.payload = std::span<const std::byte>{};
}

if (payload_type == I2cHeader::PayloadEnum::kWrite) {
callback_.i2c_write_deserialized_callback(field_id, data_view);
} else {
callback_.i2c_read_result_deserialized_callback(field_id, data_view);
}
break;
}
case I2cHeader::PayloadEnum::kReadRequest: {
if (error_flag) [[unlikely]]
co_return false;
if (data_length == 0) [[unlikely]]
co_return false;

data::I2cReadConfigView data_view{};
data_view.slave_address = slave_address;
data_view.read_length = data_length;
data_view.has_register = has_register;

if (has_register) {
const auto* register_bytes = co_await peek_bytes(1);
if (!register_bytes) [[unlikely]]
co_return false;
data_view.reg_address = std::to_integer<uint8_t>(register_bytes[0]);
consume_peeked();
}

callback_.i2c_read_config_deserialized_callback(field_id, data_view);
break;
}
case I2cHeader::PayloadEnum::kError: {
data::I2cErrorView data_view{};
data_view.slave_address = slave_address;
data_view.data_length = data_length;
data_view.has_register = has_register;
data_view.is_read = error_flag;

if (has_register) {
const auto* register_bytes = co_await peek_bytes(1);
if (!register_bytes) [[unlikely]]
co_return false;
data_view.reg_address = std::to_integer<uint8_t>(register_bytes[0]);
consume_peeked();
}
callback_.i2c_error_deserialized_callback(field_id, data_view);
break;
}
default: co_return false;
}

co_return true;
}

} // namespace librmcs::core::protocol
12 changes: 12 additions & 0 deletions core/src/protocol/deserializer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ class DeserializeCallback {

virtual void gyroscope_deserialized_callback(const data::GyroscopeDataView& data) = 0;

virtual void i2c_write_deserialized_callback(FieldId id, const data::I2cDataView& data) = 0;

virtual void
i2c_read_config_deserialized_callback(FieldId id, const data::I2cReadConfigView& data) = 0;

virtual void
i2c_read_result_deserialized_callback(FieldId id, const data::I2cDataView& data) = 0;

virtual void i2c_error_deserialized_callback(FieldId id, const data::I2cErrorView& data) = 0;

virtual void error_callback() = 0;
};

Expand Down Expand Up @@ -115,6 +125,8 @@ class Deserializer : private coroutine::InlineLifoContext<1024> {

coroutine::LifoTask<bool> process_imu_field(FieldId field_id);

coroutine::LifoTask<bool> process_i2c_field(FieldId field_id);

// Await until at least `size` contiguous bytes are available at the current read position.
// Returns a pointer to a contiguous region of at least `size` bytes.
// (from input buffer or pending cache)
Expand Down
15 changes: 15 additions & 0 deletions core/src/protocol/protocol.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,19 @@ struct ImuGyroscopePayload : utility::Bitfield<6> {
using Z = utility::BitfieldMember<32, 16, int16_t>;
};

struct I2cHeader : utility::Bitfield<3> {
enum class PayloadEnum : uint8_t {
kWrite = 0,
kReadRequest = 1,
kReadResult = 2,
kError = 3,
};

using PayloadType = utility::BitfieldMember<4, 2, PayloadEnum>;
using HasRegister = utility::BitfieldMember<6, 1>;
using ErrorFlag = utility::BitfieldMember<7, 1>;
using SlaveAddress = utility::BitfieldMember<8, 7>;
using DataLength = utility::BitfieldMember<15, 9>;
};

} // namespace librmcs::core::protocol
159 changes: 159 additions & 0 deletions core/src/protocol/serializer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,139 @@ class Serializer {
return SerializeResult::kSuccess;
}

SerializeResult write_i2c_write(FieldId field_id, const data::I2cDataView& view) noexcept {
const std::size_t required = required_i2c_size(
field_id, I2cHeader::PayloadEnum::kWrite, view.payload.size(), view.has_register);
LIBRMCS_VERIFY_LIKELY(required, SerializeResult::kInvalidArgument);
LIBRMCS_VERIFY_LIKELY(!view.payload.empty(), SerializeResult::kInvalidArgument);
LIBRMCS_VERIFY_LIKELY(view.slave_address <= 0x7F, SerializeResult::kInvalidArgument);

auto dst = buffer_.allocate(required);
LIBRMCS_VERIFY_LIKELY(!dst.empty(), SerializeResult::kBadAlloc);
utility::assert_debug(dst.size() == required);
std::byte* cursor = dst.data();

write_field_header(cursor, field_id);

auto header = I2cHeader::Ref(cursor);
cursor += sizeof(I2cHeader);
header.set<I2cHeader::PayloadType>(I2cHeader::PayloadEnum::kWrite);
header.set<I2cHeader::HasRegister>(view.has_register);
header.set<I2cHeader::ErrorFlag>(false);
header.set<I2cHeader::SlaveAddress>(view.slave_address);
header.set<I2cHeader::DataLength>(static_cast<uint16_t>(view.payload.size()));

if (view.has_register)
*cursor++ = static_cast<std::byte>(view.reg_address);

if (!view.payload.empty()) {
std::memcpy(cursor, view.payload.data(), view.payload.size());
cursor += view.payload.size();
}

utility::assert_debug(cursor == dst.data() + dst.size());
return SerializeResult::kSuccess;
}

SerializeResult
write_i2c_read_config(FieldId field_id, const data::I2cReadConfigView& view) noexcept {
const std::size_t required = required_i2c_size(
field_id, I2cHeader::PayloadEnum::kReadRequest, view.read_length, view.has_register);
LIBRMCS_VERIFY_LIKELY(required, SerializeResult::kInvalidArgument);
LIBRMCS_VERIFY_LIKELY(view.read_length != 0, SerializeResult::kInvalidArgument);
LIBRMCS_VERIFY_LIKELY(view.slave_address <= 0x7F, SerializeResult::kInvalidArgument);

auto dst = buffer_.allocate(required);
LIBRMCS_VERIFY_LIKELY(!dst.empty(), SerializeResult::kBadAlloc);
utility::assert_debug(dst.size() == required);
std::byte* cursor = dst.data();

write_field_header(cursor, field_id);

auto header = I2cHeader::Ref(cursor);
cursor += sizeof(I2cHeader);
header.set<I2cHeader::PayloadType>(I2cHeader::PayloadEnum::kReadRequest);
header.set<I2cHeader::HasRegister>(view.has_register);
header.set<I2cHeader::ErrorFlag>(false);
header.set<I2cHeader::SlaveAddress>(view.slave_address);
header.set<I2cHeader::DataLength>(view.read_length);

if (view.has_register)
*cursor++ = static_cast<std::byte>(view.reg_address);

utility::assert_debug(cursor == dst.data() + dst.size());
return SerializeResult::kSuccess;
}

SerializeResult
write_i2c_read_result(FieldId field_id, const data::I2cDataView& view) noexcept {
const std::size_t required = required_i2c_size(
field_id, I2cHeader::PayloadEnum::kReadResult, view.payload.size(), view.has_register);
LIBRMCS_VERIFY_LIKELY(required, SerializeResult::kInvalidArgument);
LIBRMCS_VERIFY_LIKELY(view.slave_address <= 0x7F, SerializeResult::kInvalidArgument);

auto dst = buffer_.allocate(required);
LIBRMCS_VERIFY_LIKELY(!dst.empty(), SerializeResult::kBadAlloc);
utility::assert_debug(dst.size() == required);
std::byte* cursor = dst.data();

write_field_header(cursor, field_id);

auto header = I2cHeader::Ref(cursor);
cursor += sizeof(I2cHeader);
header.set<I2cHeader::PayloadType>(I2cHeader::PayloadEnum::kReadResult);
header.set<I2cHeader::HasRegister>(view.has_register);
header.set<I2cHeader::ErrorFlag>(false);
header.set<I2cHeader::SlaveAddress>(view.slave_address);
header.set<I2cHeader::DataLength>(static_cast<uint16_t>(view.payload.size()));

if (view.has_register)
*cursor++ = static_cast<std::byte>(view.reg_address);

if (!view.payload.empty()) {
std::memcpy(cursor, view.payload.data(), view.payload.size());
cursor += view.payload.size();
}

utility::assert_debug(cursor == dst.data() + dst.size());
return SerializeResult::kSuccess;
}

SerializeResult write_i2c_error(FieldId field_id, const data::I2cErrorView& view) noexcept {
const std::size_t required = required_i2c_size(
field_id, I2cHeader::PayloadEnum::kError, view.data_length, view.has_register);
LIBRMCS_VERIFY_LIKELY(required, SerializeResult::kInvalidArgument);
LIBRMCS_VERIFY_LIKELY(view.slave_address <= 0x7F, SerializeResult::kInvalidArgument);

auto dst = buffer_.allocate(required);
LIBRMCS_VERIFY_LIKELY(!dst.empty(), SerializeResult::kBadAlloc);
utility::assert_debug(dst.size() == required);
std::byte* cursor = dst.data();

write_field_header(cursor, field_id);

auto header = I2cHeader::Ref(cursor);
cursor += sizeof(I2cHeader);
header.set<I2cHeader::PayloadType>(I2cHeader::PayloadEnum::kError);
header.set<I2cHeader::HasRegister>(view.has_register);
header.set<I2cHeader::ErrorFlag>(view.is_read);
header.set<I2cHeader::SlaveAddress>(view.slave_address);
header.set<I2cHeader::DataLength>(view.data_length);

if (view.has_register)
*cursor++ = static_cast<std::byte>(view.reg_address);

utility::assert_debug(cursor == dst.data() + dst.size());
return SerializeResult::kSuccess;
}

SerializeResult write_i2c_error(FieldId field_id, uint8_t slave_address) noexcept {
return write_i2c_error(field_id, data::I2cErrorView{.slave_address = slave_address});
}

private:
static constexpr bool is_i2c_field_id(FieldId field_id) { return field_id == FieldId::kI2c0; }

static constexpr bool use_extended_field_header(FieldId field_id) {
utility::assert_debug(field_id != FieldId::kExtend);
return static_cast<std::uint8_t>(field_id) > 0xF;
Expand Down Expand Up @@ -437,6 +569,33 @@ class Serializer {
return total;
}

static std::size_t required_i2c_size(
FieldId field_id, I2cHeader::PayloadEnum payload, std::size_t data_len,
bool has_register) noexcept {
LIBRMCS_VERIFY_LIKELY(is_i2c_field_id(field_id), 0);
LIBRMCS_VERIFY_LIKELY(data_len <= ((1U << 9) - 1U), 0);
switch (payload) {
case I2cHeader::PayloadEnum::kReadResult:
case I2cHeader::PayloadEnum::kError: break;
case I2cHeader::PayloadEnum::kWrite: LIBRMCS_VERIFY_LIKELY(data_len != 0, 0); break;
case I2cHeader::PayloadEnum::kReadRequest: LIBRMCS_VERIFY_LIKELY(data_len != 0, 0); break;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
default: return 0;
}

const std::size_t field_header_bytes = required_field_header_size(field_id);
const std::size_t i2c_header_bytes = sizeof(I2cHeader);
const std::size_t register_bytes = has_register ? 1 : 0;
const std::size_t payload_bytes = (payload == I2cHeader::PayloadEnum::kWrite
|| payload == I2cHeader::PayloadEnum::kReadResult)
? data_len
: 0;
const std::size_t total =
(field_header_bytes + i2c_header_bytes - 1) + register_bytes + payload_bytes;
LIBRMCS_VERIFY_LIKELY(total <= kProtocolBufferSize, 0);

return total;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

SerializeBuffer& buffer_;
};

Expand Down
Loading
Loading