Skip to content
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

Add High Power Feature for RFM69HCW #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions src/registers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,9 @@ impl core::ops::BitAnd<IrqFlags2> for u8 {
self & rhs as Self
}
}

pub enum PaOptions {
Pa0On = 0x80,
Pa1On = 0x40,
Pa2On = 0x20,
}
67 changes: 65 additions & 2 deletions src/rfm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use core::convert::TryInto;
use crate::error::{Error, Result};
use crate::registers::{
ContinuousDagc, DioMapping, DioPin, FifoMode, IrqFlags1, IrqFlags2, LnaConfig, Mode,
Modulation, Pa13dBm1, Pa13dBm2, PacketConfig, PacketFormat, Registers, RxBw, RxBwFreq,
SensitivityBoost,
Modulation, Pa13dBm1, Pa13dBm2, PaOptions, PacketConfig, PacketFormat, Registers, RxBw,
RxBwFreq, SensitivityBoost,
};
use crate::rw::ReadWrite;

Expand All @@ -25,6 +25,7 @@ pub struct Rfm69<S> {
mode: Mode,
dio: [Option<DioMapping>; 6],
rssi: i16,
tx_pwr: i8,
}

impl<S, Espi> Rfm69<S>
Expand All @@ -38,6 +39,7 @@ where
mode: Mode::Standby,
dio: [None; 6],
rssi: 0,
tx_pwr: 0,
}
}

Expand Down Expand Up @@ -271,11 +273,21 @@ where

self.reset_fifo()?;

// If the trasnmit power is over 18, turn on boost mode
if self.tx_pwr >= 18 {
Copy link
Owner

Choose a reason for hiding this comment

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

The 18 should be constant. That applies to all checks like this.

self.enable_boost_mode()?;
}

self.write_many(Registers::Fifo, buffer)?;

self.mode(Mode::Transmitter)?;
while !self.is_packet_sent()? {}

// If the trasnmit power is over 18, turn off boost mode after sending message
if self.tx_pwr >= 18 {
self.disable_boost_mode()?;
}

self.mode(Mode::Standby)
}

Expand All @@ -295,6 +307,12 @@ where
self.reset_fifo()?;

self.write(Registers::Fifo, packet_size)?;

// If the trasnmit power is over 18, turn on boost mode
if self.tx_pwr >= 18 {
self.enable_boost_mode()?;
}

self.mode(Mode::Transmitter)?;

for b in buffer {
Expand All @@ -304,6 +322,11 @@ where

while !self.is_packet_sent()? {}

// If the trasnmit power is over 18, turn off boost mode after sending message
if self.tx_pwr >= 18 {
self.disable_boost_mode()?;
}

self.mode(Mode::Standby)
}

Expand Down Expand Up @@ -353,6 +376,46 @@ where
self.write(Registers::TestLna, boost as u8)
}

pub fn tx_level(&mut self, power_level: i8) -> Result<(), Espi> {
Copy link
Owner

Choose a reason for hiding this comment

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

The function is missing documentation comment that would describe the limitation of power_level, also tx_level isn't very descriptive name, I would suggest tx_power instead WDYT?

let adjusted_power = power_level.clamp(-18, 13);
Copy link
Owner

Choose a reason for hiding this comment

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

Both -18 and 13 should be constants.


let reg_value = PaOptions::Pa0On as u8 | (adjusted_power + 18) as u8;
Copy link
Owner

Choose a reason for hiding this comment

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

18 should be constant.


self.write(Registers::PaLevel, reg_value)?;
self.tx_pwr = adjusted_power;
Ok(())
}

pub fn tx_level_high_pwr(&mut self, power_level: i8) -> Result<(), Espi> {
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above with the function documentation and the name. Also throughout the whole function, please convert all "magic" numbers into constants.

let adjusted_power = power_level.clamp(-2, 20);

let reg_value = match adjusted_power {
p if p <= 13 => PaOptions::Pa1On as u8 | (adjusted_power + 18) as u8,
p if p >= 18 => {
(PaOptions::Pa1On as u8 | PaOptions::Pa2On as u8) | (adjusted_power + 11) as u8
}
_ => (PaOptions::Pa1On as u8 | PaOptions::Pa2On as u8) | (adjusted_power + 14) as u8,
};

self.write(Registers::PaLevel, reg_value)?;
self.tx_pwr = adjusted_power;
Ok(())
}

fn enable_boost_mode(&mut self) -> Result<(), Espi> {
Copy link
Owner

Choose a reason for hiding this comment

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

All private function are defined below public ones, please move those two there as well.

self.write(Registers::Ocp, 0x0F)?;
self.write(Registers::TestPa1, 0x5D)?;
self.write(Registers::TestPa2, 0x7C)?;
Ok(())
}

fn disable_boost_mode(&mut self) -> Result<(), Espi> {
self.write(Registers::Ocp, 0x1A)?;
self.write(Registers::TestPa1, 0x55)?;
self.write(Registers::TestPa2, 0x70)?;
Ok(())
}

/// Configure Pa13 dBm 1 in corresponding register `RegTestPa1 (0x5A)`.
pub fn pa13_dbm1(&mut self, pa13: Pa13dBm1) -> Result<(), Espi> {
self.write(Registers::TestPa1, pa13 as u8)
Expand Down
48 changes: 48 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,3 +500,51 @@ fn test_is_packet_sent() {
assert!(rfm.is_packet_sent().ok().unwrap());
assert_eq!(rfm.spi.rx_buffer[0], Registers::IrqFlags2.read());
}

// Test Tx Level

#[test]
fn test_tx_level_high_pwr_min() {
// high power min is -2
let mut rfm = setup_rfm(Vec::new(), vec![]);
assert!(rfm.tx_level_high_pwr(-3).is_ok());
assert_eq!(rfm.spi.rx_buffer[0..=1], [Registers::PaLevel.write(), 0x50]);
}

#[test]
fn test_tx_level_hp_max() {
// high power max is 20
let mut rfm = setup_rfm(Vec::new(), vec![]);
assert!(rfm.tx_level_high_pwr(21).is_ok());
assert_eq!(rfm.spi.rx_buffer[0..=1], [Registers::PaLevel.write(), 0x7f]);
}

#[test]
fn test_tx_level_hp_mid() {
let mut rfm = setup_rfm(Vec::new(), vec![]);
assert!(rfm.tx_level_high_pwr(15).is_ok());
assert_eq!(rfm.spi.rx_buffer[0..=1], [Registers::PaLevel.write(), 0x7d]);
}

#[test]
fn test_tx_level_lp_min() {
// Low Power min is -18
let mut rfm = setup_rfm(Vec::new(), vec![]);
assert!(rfm.tx_level(-19).is_ok());
assert_eq!(rfm.spi.rx_buffer[0..=1], [Registers::PaLevel.write(), 0x80]);
}

#[test]
fn test_tx_level_lp_max() {
// Low power max is 13
let mut rfm = setup_rfm(Vec::new(), vec![]);
assert!(rfm.tx_level(14).is_ok());
assert_eq!(rfm.spi.rx_buffer[0..=1], [Registers::PaLevel.write(), 0x9f]);
}

#[test]
fn test_tx_level_lp_mid() {
let mut rfm = setup_rfm(Vec::new(), vec![]);
assert!(rfm.tx_level(0).is_ok());
assert_eq!(rfm.spi.rx_buffer[0..=1], [Registers::PaLevel.write(), 0x92]);
}