Added RFD900 service and also moved some tools from .\boards\argus to .\common #37
Conversation
darrenrahnemoon
left a comment
darrenrahnemoon
left a comment
There was a problem hiding this comment.
Looking really good!!
boards/argus/src/main.rs
Outdated
| #[cfg(feature = "temperature")] | ||
| static NODE_TYPE: Node = Node { | ||
| r#type: NodeType::ArgusTemperature as i32, | ||
| id: Some(0), | ||
| }; | ||
|
|
||
| #[cfg(feature = "pressure")] | ||
| static PRESSURE_SERVICE: StaticCell<AsyncMutex<argus::pressure::service::PressureService<{ AdcDevice::COUNT }>>> = StaticCell::new(); | ||
| #[cfg(feature = "pressure")] | ||
| static NODE_TYPE: Node = Node { | ||
| r#type: NodeType::ArgusPressure as i32, | ||
| id: Some(0), | ||
| }; | ||
|
|
||
| #[cfg(feature = "strain")] | ||
| static STRAIN_SERVICE: StaticCell<AsyncMutex<argus::strain::service::StrainService<{ AdcDevice::COUNT }>>> = StaticCell::new(); | ||
| #[cfg(feature = "strain")] | ||
| static NODE_TYPE: Node = Node { | ||
| r#type: NodeType::ArgusStrain as i32, | ||
| id: Some(0), | ||
| }; |
There was a problem hiding this comment.
Let's not clutter up main.rs with this. Maybe this can be placed in the node directory?
| pub enum Registers { | ||
| SerialSpeed = 1, | ||
| AirSpeed = 2, | ||
| NetId = 3, | ||
| TxPower = 4, | ||
| ECC = 5, | ||
| Mavlink = 6, | ||
| OpResend = 7, | ||
| MinFreq = 8, | ||
| MaxFreq = 9, | ||
| NumChannels = 10, | ||
| DutyCycle = 11, | ||
| LBTRSSI = 12, | ||
| RTSCTS = 13, | ||
| MaxWindow = 14, | ||
| EncryptionLevel = 15, | ||
| AntMode = 20, | ||
| } |
There was a problem hiding this comment.
Some small comments above each register as to what it does would be awesome!
There was a problem hiding this comment.
Same as AntMode. And try to not abbreviate. AntennaMode I imagine?
common/rfd900x/src/macros.rs
Outdated
| @@ -0,0 +1,41 @@ | |||
| #[macro_export] | |||
| macro_rules! rfd_ati { | |||
There was a problem hiding this comment.
I know it's probably the terminology they used but if there's a more verbose name for this that would be awesome. Hardware companies love abbreviating things because of some poor convention from the 60s so let's make things as verbose as possible :)
common/rfd900x/src/service.rs
Outdated
| const UART_BAUD: u32 = 57600; | ||
|
|
||
| // Delay (in ms) between each configuration command | ||
| const CONFIG_WRITE_DELAY_MS: u64 = 100; | ||
|
|
||
| // Offset to go from a int to a base 10 character | ||
| const ASCII_NUMBER_OFFSET: u32 = 48; | ||
|
|
||
| // Offset past the initial ATS segment in configuring the RFD900 | ||
| const ATS_OFFSET: usize = 3; | ||
|
|
||
| // Offset past the equal sign in configuring | ||
| const EQUAL_SIGN_OFFSET: usize = 1; | ||
|
|
||
| const MAX_CONFIG_PAYLOAD: usize = 16; | ||
|
|
||
| pub enum ConfigurationError { | ||
| InvalidConfig, | ||
| } | ||
|
|
||
| #[derive(Copy, Clone, Debug)] | ||
| pub struct Config { | ||
| air_speed: u8, | ||
| net_id: u8, | ||
| tx_power: u8, | ||
| ecc: bool, | ||
| mavlink: bool, | ||
| op_resend: bool, | ||
| min_freq: u32, | ||
| max_freq: u32, | ||
| num_of_channels: u8, | ||
| duty_cycle: u8, | ||
| lbt_rssi: u8, | ||
| } | ||
|
|
||
| impl Config { |
There was a problem hiding this comment.
Would be nice to take all the config related stuff to a config.rs file :)
common/rfd900x/src/data.rs
Outdated
| // DEFUALT CONSTANTS FOR CONFIGURATION | ||
| pub const AIR_SPEED_DEFAULT: u8 = 64; | ||
| pub const NET_ID_DEFAULT: u8 = 25; | ||
| pub const TX_POWER_DEFAULT: u8 = 30; | ||
| pub const ECC_DEFUALT: bool = false; | ||
| pub const MAVLINK_DEFAULT: bool = true; | ||
| pub const OP_RESEND_DEFAULT: bool = false; | ||
| pub const MIN_FREQ_DEFAULT: u32 = 915000; | ||
| pub const MAX_FREQ_DEFAULT: u32 = 928000; | ||
| pub const NUM_OF_CHANNELS_DEFAULT: u8 = 20; | ||
| pub const DUTY_CYCLE_DEFAULT: u8 = 100; | ||
| pub const LBT_RSSI_DEFAULT: u8 = 0; |
There was a problem hiding this comment.
This should go into a config.rs file. Along with the rest of your config definitions in the service :). And some more comments on them. I know it's redundant but it really helps visualize things better :)
common/rfd900x/src/service.rs
Outdated
| peri: impl Peripheral<P = T> + 'static, | ||
| tx: impl Peripheral<P = impl TxPin<T>> + 'static, | ||
| rx: impl Peripheral<P = impl RxPin<T>> + 'static, | ||
| interrupt_requests: impl Binding<T::Interrupt, InterruptHandler<T>> + 'static, | ||
| tx_dma: impl Peripheral<P = impl TxDma<T>> + 'static, | ||
| rx_dma: impl Peripheral<P = impl RxDma<T>> + 'static, |
There was a problem hiding this comment.
Maybe pass a UART service instead of passing all the params that go into constructing a UART service?
There was a problem hiding this comment.
Or actually yeah we talked about a HAL trait that has read/write, etc? That would also be awesome.
| let mut payload: [u8; MAX_CONFIG_PAYLOAD] = [0; MAX_CONFIG_PAYLOAD]; | ||
| payload[0] = b'A'; | ||
| payload[1] = b'T'; | ||
| payload[2] = b'S'; | ||
|
|
||
| let mut register_buffer = [0; 2]; | ||
| let mut value_buffer = [0; 8]; | ||
|
|
||
| let mut register_addressing_offset = 0; | ||
|
|
||
| let register_digit_offset = prepare_for_register(register_number as u32, &mut register_buffer); | ||
| let value_digit_offset = prepare_for_register(value, &mut value_buffer); | ||
|
|
||
| for i in 0..register_digit_offset { | ||
| payload[ATS_OFFSET + i] = register_buffer[i]; | ||
| } | ||
|
|
||
| payload[ATS_OFFSET + register_digit_offset] = b'='; | ||
|
|
||
| register_addressing_offset = ATS_OFFSET + register_digit_offset + EQUAL_SIGN_OFFSET; | ||
|
|
||
| for i in 0..value_digit_offset { | ||
| payload[register_addressing_offset + i] = value_buffer[i]; | ||
| } | ||
|
|
||
| payload[register_addressing_offset + value_digit_offset] = b'\r'; | ||
| payload[register_addressing_offset + value_digit_offset + 1] = b'\n'; | ||
|
|
There was a problem hiding this comment.
For this you might be better off using mutable slicer references. so something like
payload[0..3].copy_from_slice(b'ATS');
payload[3..6].copy_from_slice(address);
payload[6..10].copy_from_slice(value);
There are other ways to it too other than copy_from_slice I think.
common/rfd900x/src/service.rs
Outdated
| fn extract_ascii( | ||
| n: u32, | ||
| i: u32, | ||
| ) -> u8 { | ||
| let extract_divisor = 10_u32.pow(i); | ||
| (((n / extract_divisor) % 10) + ASCII_NUMBER_OFFSET) as u8 | ||
| } | ||
|
|
||
| /// Converts a u32 into a its ascii form, returns number of digits in array | ||
| pub fn prepare_for_register( | ||
| val: u32, | ||
| buffer: &mut [u8], | ||
| ) -> usize { | ||
| let mut len = number_of_digits(val); | ||
|
|
||
| for i in (0..len).rev() { | ||
| buffer[(len - 1) - i] = extract_ascii(val, i as u32); | ||
| } | ||
|
|
||
| return len; | ||
| } |
There was a problem hiding this comment.
So just to_string and then different indices of the string are your ascii characters
common/serial/Cargo.toml
Outdated
| "full", | ||
| ] } | ||
| embassy-embedded-hal = { version = "0.3.0" } | ||
| embassy-executor = { version = "0.7.0", features = [ |
There was a problem hiding this comment.
Once that other PR goes in can you start linking your common dependencies to the workspace as well? :)
common/serial/service.rs
Outdated
|
|
||
| pub struct SerialService { | ||
| pub uart: Uart<'static, mode::Async>, | ||
| pub node_type: Node, |
There was a problem hiding this comment.
Call it node. Also in the static cell you had as well maybe call it CURRENT_NODE. Node contains both the type of the board and the identifier of the board so having node_type might get confusing but node by itself can be a nomenclature we use for board identification.
…vices as well and add a ring buffered serial service
|
Forgive my grammar in new commit💀 |
No description provided.