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

Replace software TX/RX control with hardware PTT control #17

Closed
wants to merge 4 commits into from

Conversation

Jeak
Copy link

@Jeak Jeak commented Apr 21, 2024

Existing projects using the SA868 will be using the PTT pin for TX/RX control. I propose locking out this control via UART, and instead allowing the module to read the state of the PTT pin and enable/disable amplifiers and issue I2C commands to the RDA1846S. My implementation works by polling the state of PTT on P22 (necessary given no interrupt available on P22) and attempts to optimize performance by detecting the state of PTT, executing TX/RX changes only on that state change. Any attempts to manually set bits 5 or 6 of reg 0x30 via UART will be treated as an error.

@@ -122,7 +122,7 @@ extern const uint8_t I2C_ADDR_XCVR;
/*
* Time management
*/
void delay(uint16_t n);
void delay(uint32_t n);
Copy link
Author

@Jeak Jeak Apr 21, 2024

Choose a reason for hiding this comment

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

This was left in as an artifact of my earlier experimentation trying to make a configurable delay for people using RF switches that were triggered by PTT, and is not relevant to this PR. It should not affect functionality overall.

@silseva
Copy link

silseva commented Apr 22, 2024

Hi!
We need to keep the UART commands too, for the devices using the module and not having the PTT line wired.

@@ -447,14 +481,16 @@ bool platform_poke(uint8_t addr, uint8_t reg, uint16_t val) {

// TX requested so disable RXEN
if (reg == 0x30 && (val & (1 << 6))) {
P1_bit.no0 = 1; // P10 (RXEN) is high
//P1_bit.no0 = 1; // P10 (RXEN) is high
return false; // HW PTT override
Copy link
Author

@Jeak Jeak Apr 22, 2024

Choose a reason for hiding this comment

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

I think #17 (comment) can be addressed by simply removing the return false statements here and on line 493. Alternatively, another AT command could be added to allow hardware, software, or a combination of PTT inputs. Is there any use case where both methods would be needed?

Choose a reason for hiding this comment

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

I would recommend making it an "OR" logic. If either the serial command, OR the hardware pin are active, then put the AT1846 into transmit mode. If neither are active, then put the AT1846 into receive mode.

@edgetriggered
Copy link
Collaborator

I added this commit to my personal branch but have not tested it: 4b5f038

Is anyone able to test if this behaves well using both hardware and software PTT?

@edgetriggered
Copy link
Collaborator

Added with 4b5f038.

@SmittyHalibut
Copy link

Code review: This part of the change looked reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants