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

TypeError: can't convert LppData to int #104

Closed
scopelemanuele opened this issue Aug 23, 2021 · 11 comments · Fixed by #105
Closed

TypeError: can't convert LppData to int #104

scopelemanuele opened this issue Aug 23, 2021 · 11 comments · Fixed by #105
Labels
bug Something isn't working

Comments

@scopelemanuele
Copy link

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
Copy link
Contributor

amotl commented Sep 17, 2021

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 hiveeyes/terkin-datalogger#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
Copy link
Owner

smlng commented Sep 20, 2021

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 smlng added the bug Something isn't working label Sep 20, 2021
@smlng
Copy link
Owner

smlng commented Sep 20, 2021

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

@smlng
Copy link
Owner

smlng commented Sep 20, 2021

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
Copy link
Owner

smlng commented Sep 20, 2021

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
Copy link
Owner

smlng commented Sep 20, 2021

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
Copy link
Owner

smlng commented Sep 20, 2021

also note discussion here micropython/micropython#3158

@amotl
Copy link
Contributor

amotl commented Sep 24, 2021

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 adafruit/circuitpython#1763 and adafruit/circuitpython#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
Copy link
Contributor

amotl commented Sep 24, 2021

@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
Copy link
Owner

smlng commented Oct 12, 2021

new release 2.4.0 just up, check it out.

@amotl
Copy link
Contributor

amotl commented Nov 26, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants