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

Workaround for printf issue #207

Merged
merged 6 commits into from
May 27, 2023
Merged

Workaround for printf issue #207

merged 6 commits into from
May 27, 2023

Conversation

TMRh20
Copy link
Member

@TMRh20 TMRh20 commented May 24, 2023

Workaround for printf issue identified in issue nRF24/RF24Gateway#25

Workaround for printf issue identified in issue nRF24/RF24Gateway#25
@TMRh20 TMRh20 requested a review from 2bndy5 May 24, 2023 23:40
@TMRh20
Copy link
Member Author

TMRh20 commented May 25, 2023

Funny stuff going on with the printfs, maybe this should all be changed to use Serial.print(F(

RF24Network.cpp Outdated Show resolved Hide resolved
@2bndy5
Copy link
Member

2bndy5 commented May 25, 2023

Funny stuff going on with the printfs, maybe this should all be changed to use Serial.print(F(

That would only work on Arduino though, right?

It is wierd that the solution is to handle millis() in a separate IF_DEBUG*(). Is it possible that these debug calls happen within an ISR for RF24Gateway?

@TMRh20
Copy link
Member Author

TMRh20 commented May 25, 2023

That would only work on Arduino though, right?

Yeah, not really a good solution. I'll have to pick this up tomorrow or sometime soon.

I haven't been using RF24Gateway with the interrupts enabled because it needs root.

@2bndy5
Copy link
Member

2bndy5 commented May 25, 2023

I asked about the ISR because I think millis() (& micros()) is computed from a different memory partition. In AVR core it is computed from CPU registers. It may be done similarly in other cores that use the unified ArduinoCore-API -- all I could find is a forward declaration in that repo.

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.

The Arduino CI wasn't triggered because there were no changes to the Arduino examples. But, I'm guessing you tested this on the Arduino Nano already. All other CI is happy now, thanks!

@2bndy5
Copy link
Member

2bndy5 commented May 25, 2023

Before this gets released, we have to revert the changes in 7a7a854 and put them in a separate branch

Similarly with changes in nRF24/RF24Mesh@b774f03, they should go into a separate branch and removed from master.

Now that nrf_to_nrf is available in the Arduino Lib manager, we could also add that lib as a dependency for these network layers.

@TMRh20
Copy link
Member Author

TMRh20 commented May 25, 2023

After some thought, I think we have a bigger issue here with printf not working. Printing the millis() separately kind of works, but I am guessing its just overflowing data somewhere else because I am getting strange behaviour with debug enabled now.

- Removed all the millis() printfs
- Removed header.toString calls
@TMRh20
Copy link
Member Author

TMRh20 commented May 25, 2023

Before this gets released, we have to revert the changes...

Yeah, I would like to get this all figured out soon, we are past due for a release of all RF24 libs. Changes reverted, new branches created

@TMRh20 TMRh20 merged commit 8e444a2 into master May 27, 2023
@TMRh20 TMRh20 deleted the FixDebugPrinting branch May 27, 2023 10:22
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