-
Notifications
You must be signed in to change notification settings - Fork 97
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
feat: Add Dora-kit car Control in node-hub #715
Conversation
Added a control node for chongyoucar to receive text commands, 'forward' 'left' 'right' 'backward' and 'stop' to control the car's forward turning and backward stopping etc. Author: Leon <[email protected]>
To make this release easy to install. We can push a prebuilt version on pip. I can help with it, or you can try on your own. The idea is that you need a:
pyo3 = { workspace = true, features = [
"extension-module",
"abi3",
], optional = true }
[lib]
name = "dora_chongyoucar"
path = "src/lib.rs"
crate-type = ["lib", "cdylib"]
fn main() -> Result<(), eyre::Error> {
dora_rerun::lib_main()
}
#[cfg(feature = "python")]
use pyo3::{
pyfunction, pymodule,
types::{PyModule, PyModuleMethods},
wrap_pyfunction, Bound, PyResult, Python,
};
#[cfg(feature = "python")]
#[pyfunction]
fn py_main(_py: Python) -> PyResult<()> {
pyo3::prepare_freethreaded_python();
lib_main().map_err(|e| pyo3::exceptions::PyException::new_err(e.to_string()))
}
/// A Python module implemented in Rust.
#[cfg(feature = "python")]
#[pymodule]
fn dora_rerun(_py: Python, m: Bound<'_, PyModule>) -> PyResult<()> {
m.add_function(wrap_pyfunction!(py_main, &m)?)?;
Ok(())
} And that's it. Publishing on pip making it super easy for people to just: You can try an example with |
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.
Looks great!
{ | ||
let received_string: &str = TryFrom::try_from(&data).unwrap(); | ||
match received_string { | ||
"forward" => { |
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.
Can we use a float64
array as input instead of string so that it's easier to configure later.
It's also a more scientific representation of the action.
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.
ok, I'll readjust the technical program.
|
||
tokio::spawn(async move { | ||
while let Some((x, w)) = rx_key.recv().await { | ||
// println!("{:?}", (x, w)); |
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.
Let's remove comments.
This comment was marked as off-topic.
This comment was marked as off-topic.
I don't think we should use json. It's going to make the whole process slower and more difficult to interpolate with other components. Can it just be something in the likes of: let float64array = [x, y, rz, speed] |
Or even better, I think if we can have something in the likes of let float64array = [x, y, z, rx, ry, rz] // For position control and speed within the metadata HashMap Following http://docs.ros.org/en/noetic/api/geometry_msgs/html/msg/Twist.html You can ignore z, rx, ry. |
That's great |
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.
What is the status of this?
Can we make the requested change?
It would be great if we have short lived PR
Accepts an array of six f64's
|
…other is an array of six f64's - three f64 array [x, w, speed] - six f64 array [x, y, z, rx, ry, rz] only used x, rz, and the speed is default 0.2 https://docs.ros.org/en/noetic/api/geometry_msgs/html/msg/Twist.html Author: Leon <[email protected]>
- six f64 array [x, y, z, rx, ry, rz] only used x and rz see https://docs.ros.org/en/noetic/api/geometry_msgs/html/msg/Twist.html Author: Leon <[email protected]>
Sorry forgot to finish the review. Could we double check for the name of this package. Is chongyou-car really the name we want to use? |
|
||
#[derive(Debug, Error)] | ||
pub enum Error { | ||
#[error("serial connect fail")] |
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.
Can we make this error more descriptive. Can we add the serial port used?
This is mainly for the dora-kit car, or you could even call it the dora-kit-car. Got any better name ideas? |
That might be better yes |
Author: Leon <[email protected]>
Added a control node for chongyoucar to receive text commands, 'forward' 'left' 'right' 'backward' and 'stop' to control the car's forward turning and backward stopping etc.
Author: Leon [email protected]