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

CE set LOW twice consecutively in startConstantCarrier() #986

Open
2bndy5 opened this issue Jul 10, 2024 · 1 comment · May be fixed by #987
Open

CE set LOW twice consecutively in startConstantCarrier() #986

2bndy5 opened this issue Jul 10, 2024 · 1 comment · May be fixed by #987

Comments

@2bndy5
Copy link
Member

2bndy5 commented Jul 10, 2024

RF24/RF24.cpp

Lines 1996 to 1997 in 81348ad

ce(LOW);
reUseTX();

where the definition of reUseTX() is

RF24/RF24.cpp

Lines 1325 to 1331 in 81348ad

void RF24::reUseTX()
{
write_register(NRF_STATUS, _BV(MAX_RT)); //Clear max retry flag
write_register(REUSE_TX_PL, RF24_NOP, true);
ce(LOW); //Re-Transfer packet
ce(HIGH);
}

I think the problem is that reUseTX() doesn't deactivate the CE pin before it needs to clear the MAX_RT flag. It would make sense to me if the radio is put into standby mode before clearing the MAX_RT flag.

Git blame seems a bit confusing. The consecutive ce() calls were added in 77b1e2c, but it was closer to what I would expect beforehand in d0ec0c4 (except without clearing the MAX_RT flag)

I think this was a copy-n-paste oversight, but the changes are 10 years old 😆. FWIW, the CirPy lib does ce(LOW) -> clear flag -> ce(HIGH) as well.

Propsal

in startConstantCarrier():

- ce(LOW); 
  reUseTX();

then in reUseTX():

+     ce(LOW); //Re-Transfer packet 
      write_register(NRF_STATUS, _BV(MAX_RT)); //Clear max retry flag 
      write_register(REUSE_TX_PL, RF24_NOP, true); 
-     ce(LOW); //Re-Transfer packet 
      ce(HIGH); 

PS - I found this when writing unit tests to ensure expected behavior in pure rust. 😉

@TMRh20
Copy link
Member

TMRh20 commented Jul 10, 2024

Hmm, interesting find! I think you are probably correct in the required order of operations here.

Your rust project seems a bit ambitious, I don't know much rust, but will follow along with interest!

2bndy5 added a commit that referenced this issue Jul 15, 2024
@2bndy5 2bndy5 linked a pull request Jul 15, 2024 that will close this issue
2bndy5 added a commit that referenced this issue Aug 2, 2024
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 a pull request may close this issue.

2 participants