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

bugfix: Resolve noisy data #84

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Davidhanson90
Copy link

I am creating this PR on behalf of David Whitaker who looked into an issue with noisy data. His comments from his facebook post

Hi, I am delighted to say, I have FIXED the noisy data from sofar2mqtt.  I know many did this project and were disappointed with the data spikes.  Several suggestions about shielding and grounding, but with a CRC errors at the physical layer should never get delivered.
The first error in the .ino sketch has been noted before
  return (received_crc = calculated_crc);
should be 
  return (received_crc == calculated_crc);
The second took a little longer (2 weeks!)
Around line 380
    for(l = 0; l < sizeof(mqtt_status_reads)/sizeof(struct mqtt_status_register); l++)
        addStateInfo(state, mqtt_status_reads[l].regnum, mqtt_status_reads[l].mqtt_name);
Should be 
    for(l = 0; l < sizeof(mqtt_status_reads)/sizeof(struct mqtt_status_register); l++)
        if(addStateInfo(state, mqtt_status_reads[l].regnum, mqtt_status_reads[l].mqtt_name))
        {
            Serial.println("Error from addStateInfo");
            return;
        }
Error status was passed up 4 levels but then ignored when it mattered.
Its now running error free for 4 days.  If anyone else can replicate this fix please let me know.
Edit:- if its replicated a couple of times, then I will try to at least raise a case on Github highlighting the error so others can fix it even if the author does not amend the code.

This is for your consideration.

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.

1 participant