Fix: Update repeated field checks to use is_repeated property by pdxlocations · Pull Request #908 · meshtastic/python

Conversation

@pdxlocations

Update protobuf field-inspection logic to use is_repeated instead of label where needed.

I have not tested for backwards compatibility.

@codecov

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.84%. Comparing base (d0ccb1a) to head (cd9199b).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
meshtastic/__main__.py 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #908      +/-   ##
==========================================
+ Coverage   59.82%   59.84%   +0.01%     
==========================================
  Files          24       24              
  Lines        4329     4333       +4     
==========================================
+ Hits         2590     2593       +3     
- Misses       1739     1740       +1     
Flag Coverage Δ
unittests 59.84% <57.14%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pdxlocations

Tested with older protobuf dependencies and we'd need bump to >=6.31.0 or add a fallback.

@pdxlocations

Helper function added to test for is_repeated and fallback to label if not present.
Tested with the current minimum dependency of 4.4.21 3.20.3 and up to 7.34.0.

ianmcorvidae

Choose a reason for hiding this comment

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

Looks pretty good. Left a small documentation comment that I'll let you approve or amend first, but we can get this in and released soon hopefully.

Co-authored-by: Ian McEwen <ianmcorvidae@ianmcorvidae.net>

@ianmcorvidae

Heh, apparently my suggestion angered pylint. I'll fix that up, sorry about that.

ianmcorvidae

2 participants

@pdxlocations @ianmcorvidae