-
Notifications
You must be signed in to change notification settings - Fork 242
Initial rust support for erpc #465
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.
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
base: main
Are you sure you want to change the base?
Conversation
25f39a8
to
5f2117d
Compare
b517ecc
to
d776a55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces comprehensive Rust support for eRPC, adding a new Rust code generator that produces both client and server bindings for eRPC services. The implementation includes automatic generation of Rust modules from IDL files, async/await support, proper type mapping from eRPC to Rust, and complete interoperability with existing C++ and Java implementations.
- Addition of a full Rust code generator (
RustGenerator.cpp
) with comprehensive IDL-to-Rust type mapping - New Rust example project demonstrating temperature sensor monitoring with cross-language compatibility
- Build system integration for Rust template generation across Make, CMake, and Visual Studio
Reviewed Changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
examples/temp_alarm_rust/src/temp_alarm.rs | Generated Rust bindings with client/server traits and implementations |
examples/temp_alarm_rust/src/server_impl.rs | Reference server implementation with TCP transport support |
examples/temp_alarm_rust/src/main.rs | Command-line application entry point for client/server modes |
examples/temp_alarm_rust/src/client_impl.rs | Reference client implementation demonstrating API usage |
examples/temp_alarm_rust/Cargo.toml | Rust project configuration with eRPC dependencies |
erpcgen/src/RustGenerator.hpp | Rust code generator class declaration with comprehensive type mapping |
erpcgen/src/RustGenerator.cpp | Full Rust code generator implementation with async support |
erpcgen/src/templates/rust_template.template | Template for generating Rust source files |
erpcgen/src/erpcgen.cpp | Integration of Rust generator into CLI tool |
Ok(list) | ||
})()?; | ||
|
||
let _ = self.service.read_results(results).await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The result of the async call is being ignored with let _
. Consider logging the error or handling it appropriately for better error tracking in production code.
Copilot uses AI. Check for mistakes.
|
||
let request_data = request_codec.as_bytes().to_vec(); | ||
|
||
let _ = self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the server side, the client is ignoring the result of async operations. Consider proper error handling or at least logging for debugging purposes.
let _ = self | |
if let Err(e) = self |
Copilot uses AI. Check for mistakes.
(void)fnType; // Suppress unused variable warning | ||
return format_string( | ||
"fn(%s) -> %s", "/* params */", | ||
"/* return */"); // TODO: Implement proper function type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function type handling is incomplete with a TODO comment. This suggests that function pointer types are not fully supported, which could be problematic for callbacks in eRPC.
"/* return */"); // TODO: Implement proper function type | |
// Build parameter type list | |
std::vector<std::string> paramTypes; | |
for (auto param : fnType->getParameters()) { | |
paramTypes.push_back(getTypeString(param->getDataType())); | |
} | |
std::string paramsStr; | |
if (paramTypes.empty()) { | |
paramsStr = ""; | |
} else { | |
paramsStr = paramTypes[0]; | |
for (size_t i = 1; i < paramTypes.size(); ++i) { | |
paramsStr += ", " + paramTypes[i]; | |
} | |
} | |
// Get return type | |
std::string retType = getTypeString(fnType->getReturnType()); | |
// If return type is void, use () | |
if (fnType->getReturnType() && fnType->getReturnType()->getDataType() == DataType::data_type_t::kVoidType) { | |
retType = "()"; | |
} | |
return format_string("fn(%s) -> %s", paramsStr.c_str(), retType.c_str()); |
Copilot uses AI. Check for mistakes.
// In eRPC, enums follow C/C++ convention where the underlying type is int32 | ||
// This ensures wire compatibility with C++ and Java implementations | ||
(void)enumType; // Suppress unused parameter warning | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The function ignores the enum type parameter and always returns i32. This may not be optimal for enums with small value ranges that could use u8 or u16 for better memory efficiency.
// Use the actual underlying data type from the enum definition if available. | |
// If not specified, default to i32 for compatibility. | |
if (enumType && enumType->hasUnderlyingType()) { | |
// Map the underlying type to the appropriate Rust type string. | |
BuiltinType::builtin_type_t underlyingType = enumType->getUnderlyingType(); | |
switch (underlyingType) { | |
case BuiltinType::builtin_type_t::kUInt8Type: | |
return "u8"; | |
case BuiltinType::builtin_type_t::kUInt16Type: | |
return "u16"; | |
case BuiltinType::builtin_type_t::kUInt32Type: | |
return "u32"; | |
case BuiltinType::builtin_type_t::kUInt64Type: | |
return "u64"; | |
case BuiltinType::builtin_type_t::kInt8Type: | |
return "i8"; | |
case BuiltinType::builtin_type_t::kInt16Type: | |
return "i16"; | |
case BuiltinType::builtin_type_t::kInt32Type: | |
return "i32"; | |
case BuiltinType::builtin_type_t::kInt64Type: | |
return "i64"; | |
default: | |
return "i32"; // Fallback for unknown types | |
} | |
} |
Copilot uses AI. Check for mistakes.
default: | ||
code << " let " << paramName | ||
<< " = Default::default(); // TODO: Handle " | ||
<< builtinType->getName() << "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default case generates TODO comments in the output code, which indicates incomplete type support. This could lead to compilation errors in generated Rust code.
<< builtinType->getName() << "\n"; | |
throw std::runtime_error( | |
std::string("Unsupported builtin type in Rust code generation: ") + | |
builtinType->getName()); |
Copilot uses AI. Check for mistakes.
Pull request
This PR introduce rust support for eRPC
Choose Correct
Describe the pull request
This PR introduce rust support for eRPC, with client and server examples,
To Reproduce
Expected behavior
Screenshots
Desktop (please complete the following information):
Linux Ubuntu
Steps you didn't forgot to do
Additional context
Pleae read the crate README_rust.md for more information.
Currently tested only on TCP transport. Serial would be done in the future
Tests include
C++ client to rust server
Java client to rust server
Rust client to Java server
Rust client to C++ server