Skip to content

Version 0.7.1 Resetting REG_FIFO_ADDR_PTR in LoRaClass::handleDio0Rise #353

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

Closed
notascooby opened this issue Mar 28, 2020 · 8 comments
Closed

Comments

@notascooby
Copy link

Not receiving Lora communication (interrupt driven) with ver 0.7.1 but works perfectly with ver 0.7.0.
in ESP32 based device. Sketch & device have not been changed for over a year.
With reference to issue #218
At that time I had the same issue and commented out LINE 583 in version 0.5.0. writeRegister(REG_FIFO_ADDR_PTR, 0);
After commenting that line out, all worked perfectly. In later versions released up to and including 0.7.0 the above line of code had been removed in the commits and library worked great.
In version 0.7.1 this line of code has been re-introduced (line 680)
By commenting this line 680 out, interrupt driven receive works well.
Not sure if the line has been accidently re-introduced, or there is a reason for adding it back in, but hopefully this information may be of use.
I would prefer to use libraries as released without changes so if line 680 should be present, any pointers to the possible reasons for the issue would be gratefully received.

@morganrallen
Copy link
Collaborator

Yeah looks like this was reintroduced by PR #320
@ricaun was this a mistake or for a specific reason?

@ricaun
Copy link
Contributor

ricaun commented Mar 28, 2020

Yes it was my mistake, the PR #320 was not supposed to change the Receive part of the handleDio0Rise, just add all the original code inside the if ((irqFlags & IRQ_RX_DONE_MASK) != 0) {
I was looking whats change on the PR and what the hell I did, let's rollback this part and I gonna send a new PR fixing this part.
😢

@morganrallen
Copy link
Collaborator

No worries on rolling back, I'll just push a new change. What probably happened was you had an old version of master that didn't include this removal.

@morganrallen
Copy link
Collaborator

I've just release 0.7.2 which fixes this issue. Can take an hour or so for the release to appear, please close this issue when you think it's good @notascooby

@ricaun
Copy link
Contributor

ricaun commented Mar 28, 2020

Perfect, the result is the same. Removing the line have the same result.

@notascooby
Copy link
Author

Wow fast response - thanks.
Version 0.7.2 solves the issue.

@morganrallen
Copy link
Collaborator

heh yeah, got all the time in the world these days....
and thankfully an easy fix for something that should have been caught, I'll see if I can implement some checks to help prevent things like this is the future.

@IoTThinks
Copy link
Collaborator

@morganrallen may be you can put a comment like // PR XYZ - Resetting REG_FIFO... in front of the code line.

So when you merge, if new code doesn't have this line, you will know.
It is common in Software Development / Outsourcing.

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

No branches or pull requests

4 participants