smlng / pycayennelpp

A Cayenne Low Power Payload (CayenneLPP) decoder and encoder for Python
MIT License
20 stars 14 forks source link

TypeError: can't convert LppData to int #104

Closed scopelemanuele closed 2 years ago

scopelemanuele commented 2 years ago

Micropython Version: MicroPython v1.16 on 2021-08-23; ESP32 module with ESP32 Compiled from repository and Lpp embedded in firmware

Code:

from cayennelpp import LppFrame
frame = LppFrame()
frame.add_temperature(0, -1.2)
frame.add_humidity(6, 34.5)
buffer = bytes(frame)

Returned error:

>>> buffer = bytes(frame)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't convert LppData to int
amotl commented 2 years ago

Dear Sebastian,

we recognized your recent activity on the codebase, all accumulating into the most recent version 2.3.0, released the other day. Kudos and thanks for your excellent work!

We also would like to upgrade to the most recent release and just created a corresponding note to track this, see https://github.com/hiveeyes/terkin-datalogger/issues/103. As you might remember @thiasB and me from earlier visits to this repository, we are especially looking at support for MicroPython.

Apart from what @scopelemanuele is reporting here, we also have to support older versions of MicroPython, because Pycom's fork is not en par with MicroPython v1.16 yet.

So, I am humbly asking if you believe the issue reported here might still be a problem for us?

With kind regards, Andreas.

smlng commented 2 years ago

Hi all,

thanks @scopelemanuele for testing and reporting this issue. I assume the problem here is that MicroPython cannot handle the int(self.type) when converting a LppData to bytes. But I need to check this. If that's the case then a quick fix would be to add specific to_int() method (or similar) for LppType - or fix MicroPython. But I guess the latter would take longer 😉

Anyway, I need to verify this and then check options on how to solve this. Also, I should add specific MicroPython testing to avoid such issues in the future.

So again, thx for reporting!

smlng commented 2 years ago

@amotl could you also test this against your Pycom fork of MicroPython?

smlng commented 2 years ago

okay, I tested and found that it actually is the usage bytes(frame) which in MicroPython does not use the __bytes__ method of a class and hence fails. I will propose a PR shortly - would be very welcomed if all of you could run tests then

smlng commented 2 years ago

See #105 for a fix, I tested with the Unix port of MicroPython on MacOS. With that I was able to reproduce the bug and also test my proposed fix.

smlng commented 2 years ago

Please note important change: in MicroPython you need to use frame.to_bytes() instead of bytes(frame), the first (new version) works with MicroPython and Python3.x the latter only with Python3 - at least until MicroPython as proper support for __bytes__() methods in the future.

smlng commented 2 years ago

also note discussion here https://github.com/micropython/micropython/issues/3158

amotl commented 2 years ago

Dear Sebastian,

thank you so much for your quick reply on this issue, for submitting #105 already and for referencing related resources/discussions on the MicroPython issue tracker.

I also would like to add references to https://github.com/adafruit/circuitpython/issues/1763 and https://github.com/adafruit/circuitpython/pull/2220 by @tannewt in this context.

As I am currently traveling, it might need some time for me to be able to test your patch on real hardware. From what I read about your rationale, everything is completely fine from my perspective. I also believe testing your fix on the Unix port of MicroPython is completely sufficient. So, I believe it would not hurt to integrate this patch and cut a new release.

With kind regards, Andreas.

amotl commented 2 years ago

@amotl could you also test this against your Pycom fork of MicroPython?

I don't know for sure, but maybe @thiasB has some hardware around to give this a spin. However, he would have to test it by using a minimal example, because the other parts of the Terkin code did not yet integrate the changes to make it compatible with the most recent version of PyCayenneLPP.

smlng commented 2 years ago

new release 2.4.0 just up, check it out.

amotl commented 2 years ago

Thanks Sebastian! I have been distracted by other things, so I haven't been able to come back to this yet.