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

Set Power Amp to MIN to prevent power issue while testing #210

Closed
wants to merge 1 commit into from

Conversation

charles-plante
Copy link

Fixing #137. All communication exemple set the power to low to prevent power supply issue. It was missing in the begin() of the Mesh. I dont know if is enough to close the issue but was having the same problem and fixed it by adding that to the code.

Copy link
Member

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

I'd rather see the PA level set to min in the examples, not in the lib. This is because it is a software hack that saves on power consumption when the radio is in TX mode. Users are better off strengthening/stabilizing the power supply to the radio (often needed for PA/LNA modules).

@charles-plante
Copy link
Author

I've made more research and the power is set to HIGH in the RF24 basic lib.

setRadiation(RF24_PA_MAX, RF24_1MBPS); // LNA enabled by default

Like you said it should not be changed on the Mesh lib but in the exemple.

Problem is that we call begin() and we check the initialisation result. It will fail even if we have set radio power to MIN prior to calling the init because the setting will be overrided by the begin() function.

My sugestion is to enable power to MIN by default to prevent any power supply problem that could get frustating when testing the library.

@2bndy5
Copy link
Member

2bndy5 commented Jun 14, 2022

@TMRh20 This might require some adjustment to RF24Mesh::begin(). I was never happy with how mesh.begin() roped in radio.begin() and network.begin(), but it wasn't my call to make.

I think the most backward compatible way to address this is to add an optional arg to RF24Mesh::begin() named pa_level. I'd still prefer the default value be RF24_PA_MAX, and this way we can change the examples while keeping the lib consistent.

My suggestion is to enable power to MIN by default to prevent any power supply problem that could get frustrating when testing the library.

@charles-plante If setting the PA level to min is solving your problem, then you're having a hardware issue and need to inspect your power supply.

@TMRh20
Copy link
Member

TMRh20 commented Jun 14, 2022

I would suggest that this type of issue can and probably should be troubleshot using the RF24 core examples which set the PA level to min for testing purposes. I also think that the default should be MAX if we add an arg.

@anhnt-dev
Copy link

anhnt-dev commented Jul 24, 2022

I tried to set PA to MIN from example & inside RFMesh::begin() also, but in both cases, my node still can't connect to the master node. Am I missing something? I also tried with RFNetwork example and everything is still good.

@2bndy5
Copy link
Member

2bndy5 commented Jul 24, 2022

@anhnt-dev what driver boards are you using?

@anhnt-dev
Copy link

@anhnt-dev what driver boards are you using?

You meant the nrf module I'm using? I'm using rf24l01 pa lna

@2bndy5
Copy link
Member

2bndy5 commented Jul 24, 2022

Well the PA/LNA often requires a lot more power to transmit. That's why I was asking about the microcontroller board. Is it a RPi or some arduino-like board?

@anhnt-dev
Copy link

anhnt-dev commented Jul 25, 2022

Well the PA/LNA often requires a lot more power to transmit. That's why I was asking about the microcontroller board. Is it a RPi or some arduino-like board?

I designed a new board that uses esp8266 as a host. It's already has external power module to supply to esp & nrf module. I just wonder in RF24Network, it still can run helloworld_tx & helloworld_rx example perfectly, but when I ran RF24Mesh_Example_Master & RFMesh_Example, the node with id =1 or something else can't connect to mesh network(id = 0). It just stuck at

do {
        // mesh.renewAddress() will return MESH_DEFAULT_ADDRESS on failure to connect  
        Serial.println(F("Could not connect to network.\nConnecting to the mesh..."));    
      } while (mesh.renewAddress() == MESH_DEFAULT_ADDRESS);

@2bndy5
Copy link
Member

2bndy5 commented Jul 25, 2022

@anhnt-dev Your problem may not be power related, so this thread isn't the place to be asking questions. Please open a new issue and we can discuss it in more detail. By more detail, I need to know what board your master and child nodes are running the examples (I'll assume you have PA/LNA modules on external power sources). It would also help to know what version of the RF24* libs you are using.

@TMRh20
Copy link
Member

TMRh20 commented Sep 11, 2022

The more I think about this issue, the more I think something should be done, since power supply problems are so common. I also recently purchased some "nrf24l01" pa+lna modules that ended up coming marked as Si24R1, and cannot get them to work at high PA/LNA levels no matter what I do. Tested with multiple devices and power supplies that work fine with actual NRF chips.

My suggestion is to change the line in RF24.cpp https://github.com/nRF24/RF24/blob/master/RF24.cpp#L1088 to setDataRate(RF24_1MBPS); and to leave the radio PA level at the default (which should be max). This way users can simply call radio.begin() then radio.setPALevel(RF24_PA_MIN); before starting the mesh. This could be included in the mesh examples as well.

This way we don't have to change the RF24Mesh or RF24 APIs and we get a solution. This is the only real option I can think of that satisfies all requirements, but I think it would work well.

@2bndy5
Copy link
Member

2bndy5 commented Sep 11, 2022

leave the radio PA level at the default (which should be max).

Yes at first power up. Using setDataRate() instead of setRadiation() will also allow keeping whatever the PA Level was before a hard reset on the MCU.

I have no objection to the idea, but I just wanted to point out the difference (and why it currently uses setRadiation()).

@TMRh20
Copy link
Member

TMRh20 commented Sep 11, 2022

@2bndy5 Cool, I'll go ahead and make a PR.

@charles-plante Thanks for the suggestion here! I guess we will implement the fix at the RF24 layer. Per above, you can set the PA and LNA options prior to configuring the mesh, so I will close this PR and just make the change for the fix mentioned at the RF24 layer.

@TMRh20 TMRh20 closed this Sep 11, 2022
@2bndy5
Copy link
Member

2bndy5 commented Sep 11, 2022

@TMRh20 No need for a PR. I think this would be a simple enough change to push directly to RF24 master.

We should make a PR here (at Mesh layer) to modify the examples accordingly.

@2bndy5
Copy link
Member

2bndy5 commented Sep 11, 2022

Sorry to hear about the Si24R1 purchase. I suppose that is kinda inevitable.

I recently discovered a whole pack of unopened non-PA/LNA modules buried by my 3D printer. 🤦🏼

TMRh20 added a commit to nRF24/RF24 that referenced this pull request Sep 11, 2022
See nRF24/RF24Mesh#210
To allow users to configure the PA level in higher layers of the stack, do not set PA level in radio.begin();.
@TMRh20
Copy link
Member

TMRh20 commented Sep 11, 2022

Lol, some people find couch money, you find nrf24l01 modules...

OK, I think we only need to change the 2 main RF24Mesh examples for each platform, that way it stays consistent between devices, and the more advanced examples can use a higher PA level. And by 'we' do you mean me or you hehe?

TMRh20 added a commit that referenced this pull request Sep 11, 2022
Update examples per #210
TMRh20 added a commit that referenced this pull request Sep 11, 2022
* Update all main examples for min PA

Update examples per #210

* Update examples_RPi/RF24Mesh_Example.py

Co-authored-by: Brendan <[email protected]>

* Update examples_RPi/RF24Mesh_Example_Master.py

Co-authored-by: Brendan <[email protected]>
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