Skip to content

Conversation

TMRh20
Copy link
Member

@TMRh20 TMRh20 commented May 3, 2025

  • Utilize new stopListening() function
  • Check for MAX_PAYLOAD_SIZE on fragmented payloads

- Utilize new stopListening() function
- Check for MAX_PAYLOAD_SIZE on fragemented payloads
@TMRh20
Copy link
Member Author

TMRh20 commented May 3, 2025

This PR is dependent on the new RF24 changes not yet in place, so the build is failing

avoids allocating 64-bit addresses
@2bndy5

This comment was marked as resolved.

TMRh20 added a commit to TMRh20/nrf_to_nrf that referenced this pull request May 3, 2025
@TMRh20 TMRh20 requested a review from 2bndy5 May 3, 2025 22:23
@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

Looks like my patch to pipe_address() worked as intended, on the first try no less!
Both nodes (1 using just master branch, the other using this branch) started as network.begin(01234) print

Primary Mode            = RX
TX address              = 0x65646f4e31
pipe 0 ( open ) bound   = 0xcccccc3ecc
pipe 1 ( open ) bound   = 0x3c33ce3e3c
pipe 2 ( open ) bound   = 0x33
pipe 3 ( open ) bound   = 0xce
pipe 4 ( open ) bound   = 0x3e
pipe 5 ( open ) bound   = 0xe3

However, both nodes also show a unuxpected pipe 0 address after radio.stopListening():

Primary Mode            = TX
TX address              = 0x65646f4e31
pipe 0 ( open ) bound   = 0x0000000000

I'm guessing since my test don't actually transmit anything over the network, the TX address is never set. Still investigating...

@TMRh20
Copy link
Member Author

TMRh20 commented May 3, 2025

Yeah, I'm getting TX_ADDR = 0x0000003ccc in printDetails, RX addresses look good

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

Strange. If I hello_world_tx from this branch to master branch's hello_world_rx, then sending fails. But hello_workd_rx from this branch with master branch's hello_world_tx works fine.

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

Hmm, master branch's TX vs master branch's RX work fine. So, there must be something off with this branch's use of stopListening().

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

I think that memset() suggestion fixed it. Going to run another few tests.

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

I think the memset() suggestion specifically corrected the pipe_address() for master node.

I'm still getting no problems now.

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

I think we should let this sit until that new nrf2nrf release is picked up by the Arduino Lib Manager. We won't get a compile size report until then.

I triggered the Arduino CI to get a compile size report (via 422cd11), but the Arduino Nano 33 BLE (which uses a nRF52840) fails about conflicts with ARC named variables.

@2bndy5 2bndy5 force-pushed the Adjust4Pipe0Fix branch from 422cd11 to 101c220 Compare May 3, 2025 23:15
@TMRh20
Copy link
Member Author

TMRh20 commented May 3, 2025

I think we should let this sit until that new nrf2nrf release is picked up by the Arduino Lib Manager. We won't get a compile size report until then.

Ahh, ok, apparently that should only take an hour or so from when I published the release.

Copy link

github-actions bot commented May 3, 2025

Memory usage change @ 8fa2c48

Board flash % RAM for global variables %
arduino:avr:nano 💚 -116 - -86 -0.38 - -0.28 0 - 0 0.0 - 0.0
arduino:samd:mkrzero 💚 -56 - -48 -0.02 - -0.02 0 - 0 0.0 - 0.0
Click for full report table
Board examples/helloworld_rx
flash
% examples/helloworld_rx
RAM for global variables
% examples/helloworld_rx_advanced
flash
% examples/helloworld_rx_advanced
RAM for global variables
% examples/helloworld_tx_advanced
flash
% examples/helloworld_tx_advanced
RAM for global variables
% examples/helloworld_tx
flash
% examples/helloworld_tx
RAM for global variables
% examples/Network_Priority_RX
flash
% examples/Network_Priority_RX
RAM for global variables
% examples/Network_Priority_TX
flash
% examples/Network_Priority_TX
RAM for global variables
%
arduino:avr:nano -86 -0.28 0 0.0 -106 -0.35 0 0.0 -116 -0.38 0 0.0 -102 -0.33 0 0.0 -104 -0.34 0 0.0 -116 -0.38 0 0.0
arduino:samd:mkrzero -48 -0.02 0 0.0 -48 -0.02 0 0.0 -56 -0.02 0 0.0 -56 -0.02 0 0.0 -48 -0.02 0 0.0 -56 -0.02 0 0.0
Click for full report CSV
Board,examples/helloworld_rx<br>flash,%,examples/helloworld_rx<br>RAM for global variables,%,examples/helloworld_rx_advanced<br>flash,%,examples/helloworld_rx_advanced<br>RAM for global variables,%,examples/helloworld_tx_advanced<br>flash,%,examples/helloworld_tx_advanced<br>RAM for global variables,%,examples/helloworld_tx<br>flash,%,examples/helloworld_tx<br>RAM for global variables,%,examples/Network_Priority_RX<br>flash,%,examples/Network_Priority_RX<br>RAM for global variables,%,examples/Network_Priority_TX<br>flash,%,examples/Network_Priority_TX<br>RAM for global variables,%
arduino:avr:nano,-86,-0.28,0,0.0,-106,-0.35,0,0.0,-116,-0.38,0,0.0,-102,-0.33,0,0.0,-104,-0.34,0,0.0,-116,-0.38,0,0.0
arduino:samd:mkrzero,-48,-0.02,0,0.0,-48,-0.02,0,0.0,-56,-0.02,0,0.0,-56,-0.02,0,0.0,-48,-0.02,0,0.0,-56,-0.02,0,0.0

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

I just removed that board from the CI (hack).

image

Now that's what I was expecting! What a great feeling of achievement.

@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

ok So, once that nrf2nrf release get picked up, I'll revert the CI changes and merge this.

Note

This lib would only get a patch bump.

@2bndy5 2bndy5 force-pushed the Adjust4Pipe0Fix branch from 8fa2c48 to d12ca68 Compare May 3, 2025 23:57
@2bndy5
Copy link
Member

2bndy5 commented May 3, 2025

image
Its live.

I also adjusted the minimum version of RF24 and nrf2nrf in PIO metadata.

@2bndy5 2bndy5 merged commit a7f2362 into master May 4, 2025
62 checks passed
@2bndy5
Copy link
Member

2bndy5 commented May 4, 2025

Ok, I'm going to do that release crusade.

@2bndy5 2bndy5 deleted the Adjust4Pipe0Fix branch May 4, 2025 00:07
2bndy5 added a commit that referenced this pull request May 4, 2025
- Utilize new stopListening() function
- Check for MAX_PAYLOAD_SIZE on fragemented payloads
- `pipe_address()` use buffer by reference
  avoids allocating 64-bit addresses
- adjust min PIO deps' versions

---------

Co-authored-by: Brendan <[email protected]>
@2bndy5
Copy link
Member

2bndy5 commented May 4, 2025

I just got done cherry picking the fixes since last v2.x release to v1.x branch.

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.

2 participants