parallaxinc / PropLoader

Parallax Propeller loader supporting both serial and wifi downloads
MIT License
27 stars 13 forks source link

Add logic and message to alert when communication lost #39

Closed PropGit closed 7 years ago

PropGit commented 7 years ago

[This request created in response to Issue #27]

In much the same way that the added "Propeller not found" error message clarifies download failures, we need a Communication lost message.

This should apply to both wired and wireless downloads and should appear before the "ERROR: Download failed: -1" message so that instead of something like this:

001-Opening file '../myfile.binary'
002-Downloading file to port ...
009-... bytes sent
003-Verifying RAM
102-ERROR: Download failed: -1

we see this (assuming no response was received for the RAM checksum and assuming the error code is 126):

001-Opening file '../myfile.binary'
002-Downloading file to port ...
009-... bytes sent
003-Verifying RAM
126-ERROR: Communication lost
102-ERROR: Download failed: -1
dbetz commented 7 years ago

What about other reasons for the protocol failing? Currently, the TransmitPacket function can fail if it tries 3 times and each attempt gets a timeout. It can also fail if it gets data but the packet ID or tag is wrong. Do we want errors for each of those cases?

PropGit commented 7 years ago

Yes, other reasons for protocol failing at that point were intended to be covered by this issue too. For the timeout cases, they can result in the example I gave "126-ERROR: Communication lost."

Actually, even the case where it gets the wrong packet ID or the tag is wrong... those can also use the same "126-ERROR: ..." result. If we need to know more, we can use the verbose option to see that it did indeed receive something, but it was an unexpected result.

This way, we don't need 10 new error codes (1 to cover each case), but perhaps much fewer (1, 2 or 3 maybe) that nicely covers all cases. Let me know if you question certain error cases and what error to emit.

NOTE: 126 was just the next error code at the time of this issue's creation; it should be set to the proper value when created.

dbetz commented 7 years ago

I currently have three errors that could fall into this category:

RX handshake timeout RAM checksum timeout EEPROM checksum timeout

Do you want all of these to produce the "Communication lost" error?

dbetz commented 7 years ago

done

PropGit commented 7 years ago

Did you implement each of those three with a single "Communication lost" error? That was the intent, yes.

PropGit commented 7 years ago

Verified on v1.0-37.