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

Module cannot handle times between 1:05 and 0:57 seconds #7

Closed
betterengineering opened this issue Nov 17, 2015 · 7 comments
Closed

Comments

@betterengineering
Copy link
Contributor

_ser.readline() is being returned early when the time_remaining is somewhere between 1.1 and 0.9, causing the temperature read to be incorrect and the roaster to stop roasting.

@kmorey
Copy link
Contributor

kmorey commented Nov 17, 2015

I may have a fix for it... waiting for my tester to reply. Happy to do a PR when confirmed working.

@kmorey
Copy link
Contributor

kmorey commented Nov 17, 2015

If you want to try it out, here is what I came up with:

def _read_next(self):
    r = b''
    safety = 50
    while safety > 0:
        safety -= 1
        r += self._ser.read()
        if r[-2:] == self._footer:
            # have read up to and including the footer
            if len(r) == 14:  # good packet
                return r
            else:  # partial packet, but have read to a boundary... try again
                return self._read_next()

Replace any self._ser.readline() with self._read_next(). Note: I haven't tested this at all so treat it as pseudocode. It has the potential to recurse infinitely under certain conditions so might want a way to bust out of that in the final version.

@betterengineering
Copy link
Contributor Author

I like what you are doing in checking for the footer. Pyserial also allows us to read a specified number of bytes with self._ser.read(14), which is what it looks like you are trying to achieve. This sounds like a good solution, reading in 14 bytes and checking for a footer.

@kmorey
Copy link
Contributor

kmorey commented Nov 17, 2015

Yes, I considered that but I didn't want to have to keep the extra bytes around if I over-read the buffer. Say somehow we got out of sync when reading and we started always on a different byte than 1, we'd be throwing away every other packet unless we kept it around in memory and appended to it each time and then just took the 14 bytes ending in the footer.

@betterengineering
Copy link
Contributor Author

Ah, I see. I have usually leaned towards throwing away a packet if it meant the communication to continue with minimal interruption. There were times in the past that we had attempted to wait for proper packets, and it had rather negative side effects (the thermostat did wonky things, the graph had flat lines, etc). It would inevitably only affect one packet (the bad one), as there is a timeout value on the read of .25 seconds. In addition, the roaster will not send another packet until it has received a packet. But I do like your idea of not trashing the packet if it could have been salvaged and doing it in a way that does not have negative side effects.

@betterengineering
Copy link
Contributor Author

I apologize Kevin, after looking this over this morning I realized you were going to submit a pull request and I jumped the gun. In my tiredness I assumed I was supposed to implement the code because it was pasted here, but you were simply pasting it here for me to test it as well.

@kmorey
Copy link
Contributor

kmorey commented Nov 17, 2015

No worries! I posted it here for both reasons, really. I don't have a device to test with, so it may be a while before I can get it working reliably. I do have some concerns with what you ended up with, but we can figure it out iteratively once it gets tested.

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

2 participants