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

Validate checksum and retry #35

Merged

Conversation

false-git
Copy link
Contributor

Discussions にコメントいただいた点を修正してPRを出してみます。
修正すべき点があったらご指摘ください。

Copy link
Owner

@UedaTakeyuki UedaTakeyuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add any new function.

I think it can be more simple just like as follows:

      if p_ver == '2':
        if len(s) >= 4 and s[0] == "\xff" and s[1] == "\x86" and checksum(s[0:-1]) == s[-1]:
          return {'co2': ord(s[2])*256 + ord(s[3])}
        break
      else:
        if len(s) >= 4 and s[0] == 0xff and s[1] == 0x86 and checksum(s[0:-1]) == s[-1]:
          return {'co2': s[2]*256 + s[3]}
        break

Would you please try like above? thx!

@false-git
Copy link
Contributor Author

python3 のコードは以下のように変えれば動きます。(チェックサムを求める範囲を[1:-1]にして、ord()をかける)

        if len(s) >= 4 and s[0] == 0xff and s[1] == 0x86 and ord(checksum(s[1:-1])) == s[-1]:
          return {'co2': s[2]*256 + s[3]}

しかし、python2の方は、s[1:-1]checksum()に渡してもsum()の計算でエラーになります。

@UedaTakeyuki
Copy link
Owner

python2の方は、s[1:-1] を checksum()に渡してもsum()の計算でエラーになります。

Yes. You are right.
Would you please fix also checksum()?
Or, do you mind if I will fix myself?

@UedaTakeyuki
Copy link
Owner

UedaTakeyuki commented Nov 8, 2021

Followings are also sloppy work, but working well on both p2 and p3.

      if p_ver == '2':
        if len(s) >= 4 and s[0] == "\xff" and s[1] == "\x86" and ord(checksum(s)):
          return {'co2': ord(s[2])*256 + ord(s[3])}
        break
      else:
        if len(s) >= 4 and s[0] == 0xff and s[1] == 0x86 and checksum(s):
          return {'co2': s[2]*256 + s[3]}
        break

and

def checksum(array):
  if p_ver == '2':
    summation = 0
    for c in array:
      summation += ord(c)
    return struct.pack('B', 0xff - (summation % 0x100) + 1)
  else:
    return struct.pack('B', 0xff - (sum(array) % 0x100) + 1)

To be frank, the latter one seems slightly clumsy, so I'm eager for your nicer fix, ths!

@false-git false-git force-pushed the validate_checksum_and_retry branch from 7a55420 to acbc523 Compare November 8, 2021 11:52
@false-git
Copy link
Contributor Author

checksumを以下のように修正し、他の部分の修正もなるべく元の形に近いようにしてみました。

def checksum(array):
  if p_ver == '2':
    array = [ord(c) for c in array]
  return struct.pack('B', 0xff - (sum(array) % 0x100) + 1)

@UedaTakeyuki
Copy link
Owner

I really appreciate your contribution!

@UedaTakeyuki UedaTakeyuki merged commit 660693a into UedaTakeyuki:master Nov 8, 2021
@UedaTakeyuki
Copy link
Owner

Thank you!

@UedaTakeyuki
Copy link
Owner

I'll soon update the version on PyPI (after dinner, sorry).

@false-git
Copy link
Contributor Author

すいません、元々checksum()を呼んでいた span_point_calibration()がpython2で正しく動かないことに気づいたので、修正したものをpushしました。
既にmerge済みなので、お手数ですが PyPl 対応をするときに一緒に修正をお願いできますか?

def checksum(array):
  if p_ver == '2' and isinstance(array, str):
    array = [ord(c) for c in array]
  return struct.pack('B', 0xff - (sum(array) % 0x100) + 1)

@UedaTakeyuki
Copy link
Owner

sure

@UedaTakeyuki
Copy link
Owner

I've updated the Pypi. have fun!

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

Successfully merging this pull request may close these issues.

2 participants