yozik04 / nextion

Nextion serial client
GNU Lesser General Public License v3.0
25 stars 10 forks source link

Error encountered when negative integer is received by Python #18

Closed elepsistech closed 3 years ago

elepsistech commented 3 years ago

Hi Jevgeni, First of congratulations on creating such a nice and easy library to use with Nextion HMI. I have been using it, and very pleased to find it.

Recently, encountered an issue when receiving negative value from the HMI. Below is the stack trace of the error. Hopefully the fix will be the part of the next release.

Once again thanks for such a nice library.

Best Regards, Ashish

2020-12-18 20:07:37,983 - DEBUG - sent: b'get device_page.val' 2020-12-18 20:07:37,994 - DEBUG - received: b'71a5' 2020-12-18 20:07:37,995 - DEBUG - received: b'' 2020-12-18 20:07:37,996 - ERROR - Task exception was never retrieved future: <Task finished coro=<device() done, defined at next.py:114> exception=error('unpack requires a buffer of 4 bytes')> Traceback (most recent call last): File "next.py", line 159, in device page = await nex.get("device_page.val") File "/home/pi/aros_py/nextion/client.py", line 227, in get return await self.command("get %s" % key, timeout=timeout) File "/home/pi/aros_py/nextion/client.py", line 351, in command return await self._command(command, timeout=timeout, attempts=attempts) File "/home/pi/aros_py/nextion/client.py", line 323, in _command data = struct.unpack("i", raw)[0] struct.error: unpack requires a buffer of 4 bytes

yozik04 commented 3 years ago

Hi Ashish, Hah, never tried negative values. Will have a look near days.

elepsistech commented 3 years ago

Hi Jevgeni, No worries, glad to find the bug, I hope it will make the library more rounded. I have been using it for last 7-8 days and will be using it for next 1 month more. Will report any other bugs, as I encounter. Best Regards, Ashish

yozik04 commented 3 years ago

It is very strange that you got only 1 byte of payload instead of 4.

yozik04 commented 3 years ago

It is possible that you got 0x71 0xa5 0xFF 0xFF 0xFF 0xFF 0xFF 0xFF payload. Which is:

struct.unpack("i", b'\xa5\xff\xff\xff')[0]
-91

In this case my stupid simple approach will not work and I will have to rewrite it to actually pay attention to expected command response length.

elepsistech commented 3 years ago

Hi Jevgeni,

Sorry for my late reply, was off for dinner. Yeah, you are totally right, that is what is happening. Can I propose an ugly solution? If the "leftover" is same as EOL, concatenate "message" and "leftover". I am not sure, will it affect other areas, anyway, it is very ugly solution. I will live it up to the master to fix it properly.

Best Regards, Ashish

yozik04 commented 3 years ago

That will not work. It can come in random length chunks.

elepsistech commented 3 years ago

Hi again,

Oh I see, says the blind. In that case data_received() will need an overhaul.

Best Regards, Ashish

elepsistech commented 3 years ago

Hi Jevgeni,

Do you think, instead of using partition(), rpartition(), should fix it.

Best Regards, Ashish

yozik04 commented 3 years ago

rpartition will spoil all. You can have multiple replies in the buffer. Response from a command and an event for example.

elepsistech commented 3 years ago

Hi Javegni,

Thanks for letting me know. I will let the expert deal with the issue, rather than making silly suggestions.

Best Regards, Ashish

elepsistech commented 3 years ago

Hi Javegni,

Will this patch in client.py, around line # 300, work?

                elif type_ == ResponseType.NUMBER:  # number
                    raw = raw + b'\xff\xff\xff' if len(raw) == 1 else raw
                    data = struct.unpack("i", raw)[0]

Best Regards, Ashish

yozik04 commented 3 years ago

That will ruin next message. What if payload will be: 0x71 0xa5 0xa5 0xa5 0xFF 0xFF 0xFF 0xFF?

elepsistech commented 3 years ago

Hi Javegni,

Good morning to you. Thanks for your reply and pointing me the pitfall. Hopefully the below has none.

                elif type_ == ResponseType.NUMBER:  # number
                    raw_length = 4 - len(raw)
                    for counter in range(0, raw_length):
                        raw += b'\xff'
                    data = struct.unpack("i", raw)[0]

Best Regards, Ashish

yozik04 commented 3 years ago

Proper software is not written that way. Test is written first. I am in progress of fixing it. Parsing will be smarter.

elepsistech commented 3 years ago

Hi Javegni,

What would you expect from a rookie programmer :). Glad you are working on it, to fix it.

Please do update me, once it is done. Thanks in advance.

Best Regards, Ashish

yozik04 commented 3 years ago

Should be fixed now. Please verify and I will make a release then.

yozik04 commented 3 years ago

1.7.1 was released.

elepsistech commented 3 years ago

Hi Javegni,

Good morning, hope you had a good weekend. That is excellent stuff. Will check it and will let you know, how it goes.

Once again, thanks for fixing it so soon.

Best Regards, Ashish