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

RF24: Add 4-pin configuration support #1381

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

fabyte
Copy link

@fabyte fabyte commented Jan 9, 2020

Running the radio in a 4-pin configuration (MISO, MOSI, SCK, CSN) with
CE pin pulled to VCC, requires the driver to switch to Power Down
state before entering TX or RX mode.
See product specification section 6.1.1 "State diagram".

For the 4-pin configuration, MY_RF24_CE_PIN has to be set to
NOT_A_PIN.

Running the radio in a 4-pin configuration (MISO, MOSI, SCK, CSN) with
CE pin pulled to VCC, requires the driver to switch to Power Down
state before entering TX or RX mode.
See product specification section 6.1.1 "State diagram".

For the 4-pin configuration, MY_RF24_CE_PIN has to be set to
NOT_A_PIN.
@fabyte fabyte mentioned this pull request Jan 9, 2020
@Yveaux
Copy link
Member

Yveaux commented Jan 11, 2020

There are many more places in the nRF24 driver that make use the CE-pin, e.g the RF24_sleep() function that you now call instead. This patch does not seem complete.

@fabyte
Copy link
Author

fabyte commented Jan 12, 2020

You're right. So there are 3 ways to approach this issue:

  1. Surround each RF24_ce() call with a preprocessor guard like in this patch:
#if MY_RF24_CE_PIN != NOT_A_PIN
	RF24_ce(HIGH);
#endif
  1. Surround the hwDigitalWrite() call in RF24_ce with the preprocessor guard from above:
LOCAL void RF24_ce(const bool level)
{
#if MY_RF24_CE_PIN != NOT_A_PIN
	hwDigitalWrite(MY_RF24_CE_PIN, level);
#endif
}
  1. Let hwDigitalWrite (and in conclusion the hardware dependant digitalWrite() implementation) handle the pin assignment. For example, the AVR lib returns if the given pin is NOT_A_PIN.

Which should it be? I prefer option 2.

@Yveaux
Copy link
Member

Yveaux commented Jan 12, 2020

But the CE pin has a function in the nRF24 communication protocol. Will the nRF24 still function correctly in all cases when tied to VCC?

@fabyte
Copy link
Author

fabyte commented Jan 15, 2020

According to the specification, the CE pin is used to activate the chip in RX/TX mode. Having a look at the state diagram in chapter 6.1.1, you can see that this pin is only used to make the transition from Standby-I to TX/RX Mode (including additional transition states) and from these modes back to Standby-I by setting CE low. So if the only use of CE low is to get back to Standby-I and as an alternative it is possible to go there via the Power Down state, I believe this covers all functionality.

I tested this method and it works in my setup in both RX and TX modes. Do you have other cases in mind that I didn't see?

One thing I was missing is that in RF24_initialize() the pinMode should only be set in case the CE pin is not NOT_A_PIN. I will fix this as soon as we know how to go forward with this patch.

@Yveaux
Copy link
Member

Yveaux commented Jan 18, 2020

Looking at the nRF24L01+ datasheet ("nRF24L01+ Single Chip 2.4GHz Transceiver Product Specification v1.0") it contains a number of references to toggling the CE pin, or setting it low:

6.1.5 TX mode
... To enter this mode, the nRF24L01+ must have the PWR_UP bit set high, PRIM_RX bit set low, a payload in the TX FIFO and a high pulse on the CE for
more than 10µs.

6.4 Received Power Detector measurements
... If no packets are received the RPD is
latched at the end of a receive period as a result of host MCU setting CE low or RX time out controlled by Enhanced ShockBurst™.

7.4.2 Auto Retransmission (ART)
... The MCU must initiate each transmission of the packet with a pulse on the CE pin when this command is used.

ART isn't used by MySensors, and RPD is an optional function, so we can do without. However references to packet transmission always indicate a pulse on the CE pin, which isn't possible when CE is tied to VCC.
Although not perfectly clear, it suggests you're using the nRF24L01+ out of spec. Especially with all the clones and variants on the market, I can't say if this solution will work in all cases.

@fabyte
Copy link
Author

fabyte commented Jan 18, 2020

Yeah, this configuration is definitely using the radio out of spec. It is meant to be an option for special designs with the understanding that probably not all (optional) features are available or only limited available.
One could make this more clear with a #warning, or implement a feature toggle like #define MY_RF24_4_PIN_CONFIGURATION and describe the limits in the feature description.

@fabyte
Copy link
Author

fabyte commented Mar 12, 2020

@Yveaux I would like to finish up this pull request. My plan would be:

  1. Implement the feature toggle MY_RF24_4_PIN_CONFIGURATION with a description that describes the limits and also that the radio is used out of spec. This toggle sets MY_RF24_CE_PIN to NOT_A_PIN
  2. Using option 2 from above, check if ce pin is NOT_A_PIN and return in RF24_ce()
  3. Don't set pinMode in RF24_initialize

Do you agree to proceed with the pull request this way?

@fiskn
Copy link

fiskn commented Oct 31, 2020

@fabyte Thanks for this patch. I've got a ESP8266 gateway which I also needed to control a number of relays and direct attached ds18b20 sensors. Due to shortage of pins, this allowed me to not require the CE pin and build my gateway. Everything seems to be working great.

@fabyte
Copy link
Author

fabyte commented Nov 2, 2020

@fiskn Thanks for your feedback, good to hear that this solution works for you :)

@dfleck
Copy link

dfleck commented Sep 11, 2021

Could fabyte perhaps fix the build errors? I would also like this capability.

@dfleck
Copy link

dfleck commented Sep 11, 2021

Never mind. I see many pull requests blocked at "Toll gate (STM32F1 - Tests) — Warnings found". This makes me believe the mainline is broken which is causing pull requests on any forks to fail.

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

Successfully merging this pull request may close these issues.

4 participants