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

CC1101::receive will always return ERR_RX_TIMEOUT #348

Closed
asafteirobert opened this issue Aug 12, 2021 · 6 comments
Closed

CC1101::receive will always return ERR_RX_TIMEOUT #348

asafteirobert opened this issue Aug 12, 2021 · 6 comments
Labels
bug Something isn't working resolved Issue was resolved (e.g. bug fixed, or feature implemented)

Comments

@asafteirobert
Copy link

Describe the bug
CC1101::receive will always return ERR_RX_TIMEOUT.
This is because GDO0 will always stay high after a packet is received.
I have tracked it down to this change.
https://github.com/jgromes/RadioLib/pull/279/files#diff-8f4be18065fcaf0dcbf0cc902df2a1669a8dc78b86f344d2c0a3804d09b8240dR331

CC1101_REG_IOCFG0 was changed from CC1101_GDOX_SYNC_WORD_SENT_OR_RECEIVED (0x06) to CC1101_GDOX_RX_FIFO_FULL_OR_PKT_END (0x01) which seems to be causing the issue.

According to the datasheet 0x01 "De-asserts when the RX FIFO is empty." it says nothing about de-asserting on packet end. Maybe there was some confusion from the name and comment for CC1101_GDOX_RX_FIFO_FULL_OR_PKT_END?

The behavior of 0x01 causes CC1101::receive to time out while waiting for packet end, since GDO0 won't actually de-assert on packet end.

To Reproduce
Run CC1101_Transmit.ino and CC1101_Receive.ino from the examples. CC1101_Receive.ino won't print Data: Hello World! as expected.
Replacing CC1101_GDOX_RX_FIFO_FULL_OR_PKT_END with CC1101_GDOX_SYNC_WORD_SENT_OR_RECEIVED in CC1101::startReceive causes the Hello World packets to be received, but I'm also receiving a lot of junk data, so the fix is likely more complicated.

Expected behavior
When running CC1101_Transmit.ino and CC1101_Receive.ino, CC1101_Receive.ino should correctly receive the hello world packets and not anything else.

Additional info:

  • MCU: Arduino pro mini transmitter, ESP32 receiver
  • Wireless module type: CC1101 (likely clone).
  • Library version 4.5.0
  • Possibly related issue: CC1101 + ESP32 receiving random junk data #345. That issue also mentions CC1101_Receive returning error -6.
@asafteirobert
Copy link
Author

@Guglio95

@aguglie
Copy link
Contributor

aguglie commented Aug 13, 2021

Hello @asafteirobert,
Unfortunately, I no longer own the development board so I can't work on it.

As the datasheet states, it de-asserts when the RX FIFO is empty; the aim was to keep reading from RX_FIFO until it gets empty (IRQ de-asserted) and return the data.

It's true that the datasheet says nothing about de-asserting on the packet end, but it should be de-asserted since the driver read all the packet bytes.

@asafteirobert
Copy link
Author

asafteirobert commented Aug 13, 2021

I'm talking about the simple blocking CC1101::receive. Are there interrupts I'm not seeing?
Though my debugging this is what happens:

  1. CC1101::receive is called.
  2. Here it waits for GDO0 to go high.
  3. After GDO0 goes high, it now immediately waits for GDO0 to go low.
  4. Since nothing is reading from RX_FIFO, GDO0 never goes low, so CC1101::receive just returns ERR_RX_TIMEOUT

@aguglie
Copy link
Contributor

aguglie commented Aug 13, 2021

Hello @asafteirobert
I think you are right, the code-block (L161-169) is odd: it waits for GD0 to go low but it will go low only after the data is read (L172).

The (working) revision I was using did non contain that waiting-part.

Did you try to get rid of it?

@jgromes
Copy link
Owner

jgromes commented Aug 15, 2021

I think you are right, the code-block (L161-169) is odd: it waits for GD0 to go low but it will go low only after the data is read (L172).

I remember writing that in one of the very first versions of the CC1101 driver, when (as @asafteirobert correctly pointed out), CC1101_GDOX_SYNC_WORD_SENT_OR_RECEIVED was used in blocking reception. After the first GDO level change on sync word detection, there was another at the end of packet reception, so that's why there are two waiting loops.

Digging through the commit history, it seems like that what happened was that there were to PRs from @Guglio95 (/ the RFQuack team as a whole):

  1. [CC1101] Receive and Transmit up to 255 bytes #114, which was closed without merging due to an unresolved bug in CRC handling. This is where the change from two waiting loops to single one happened. Therefore, after this point, the original two waiting loops remained in RadioLib.

  2. New .gitignore pattern, CC1101 streaming mode, RF69 SPI get/set/read/write overrides #279, which was merged. This however was based on the same branch of rfquack as [CC1101] Receive and Transmit up to 255 bytes #114, so it contained the old changes which then propagated throughout RadioLib master. You can see commits from [CC1101] Receive and Transmit up to 255 bytes #114 in RadioLib master commit history, even though that PR was not merged.

So long story short, I should probably do more thorough reviews of PRs.

@jgromes
Copy link
Owner

jgromes commented Aug 18, 2021

@asafteirobert it seems that presence of a second waiting loop is indeed the cause of blocking receive always returning ERR_RX_TIMEOUT, removing it solved this particular issue. Though it still seems that sometimes, CC1101 receives garbage packets as reported in #345, so this is unrelated.

Should be fixed now, thanks for looking into this!

@jgromes jgromes closed this as completed Aug 18, 2021
@jgromes jgromes added bug Something isn't working resolved Issue was resolved (e.g. bug fixed, or feature implemented) labels Aug 18, 2021
phretor added a commit to rfquack/RadioLib that referenced this issue Sep 16, 2021
* 'master' of git://github.com/jgromes/RadioLib: (83 commits)
  [nRF24] Added interrupt-driven examples
  [AX.25] Added option to adjust audio frequencies (jgromes#346)
  [MQTT] User SerialModule wrapper
  [HTTP] User SerialModule wrapper
  [XBee] Use SerialModule wrapper
  [JDY08] Use SerialModule wrapper
  [HC05] Use SerialModule wrapper
  Added SerialModule wrapper class (jgromes#305)
  [CC1101] Fixed blocking receive always returning timeout (jgromes#348)
  [CC1101] Added 0x17 as valid version number (jgromes#349)
  Added AFSK via OOK example
  Bump version to 4.5.0
  [Si443x] Fixed rxosr calculation (jgromes#199)
  [Si443x] Added antenna switching on GPIO0/1
  [Si443x] Fixed preamble configuration (jgromes#199)
  [Si443x] Added software reset
  [PHY] Fixed negative random numbers (jgromes#328)
  [RF69] Renamed TRNG method
  [RF69] Renamed TRNG method
  [SX126x] Renamed TRNG method
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resolved Issue was resolved (e.g. bug fixed, or feature implemented)
Projects
None yet
Development

No branches or pull requests

3 participants