Skip to content

Conversation

@JMcrafter26
Copy link

No description provided.

…s.md with Flipper Zero instructions, add decodeValues.py for block decoding, and create requirements.txt for dependencies
Copilot AI review requested due to automatic review settings December 22, 2025 23:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the project's Python tooling and documentation for working with Bambu Lab RFID tags. The changes standardize import paths for cryptographic dependencies, add comprehensive instructions for Flipper Zero users, introduce a new decoder utility for parsing tag data, and establish proper Python project conventions.

Key Changes:

  • Fixed import paths in deriveKeys.py to use the standard Crypto module naming convention
  • Added detailed Flipper Zero workflow documentation in ReadTags.md
  • Created decodeValues.py script for decoding and interpreting RFID tag block data

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
requirements.txt New file specifying pycryptodome as project dependency
docs/ReadTags.md Added comprehensive Flipper Zero usage instructions with step-by-step workflow
deriveKeys.py Updated imports from Cryptodome to Crypto namespace with inline installation comment
decodeValues.py New utility for decoding NFC dump files with typed field parsing for documented blocks
.gitignore Added entries for .nfc files and pycache directory

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

import sys
from Cryptodome.Protocol.KDF import HKDF
from Cryptodome.Hash import SHA256
from Crypto.Protocol.KDF import HKDF # pip install pycryptodome
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The inline comment 'pip install pycryptodome' is redundant since requirements.txt now documents this dependency. Consider removing the inline comment to avoid duplication.

Suggested change
from Crypto.Protocol.KDF import HKDF # pip install pycryptodome
from Crypto.Protocol.KDF import HKDF

Copilot uses AI. Check for mistakes.


if __name__ == '__main__':
import sys
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The sys module is imported twice: once at line 2 and again at line 337. Remove the duplicate import at line 337.

Suggested change
import sys

Copilot uses AI. Check for mistakes.
# Color RGBA (0..3), weight uint16 LE at 4, diameter float LE at 8
rgba = ''.join(f"{(b or 0):02X}" for b in bytes_list[0:4])
weight = get_uint16_le(bytes_list, 4)
diameter = get_float32_le(bytes_list, 8)
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The slicing operation [:-2] removes the alpha channel without explanation. Add a comment explaining why the alpha channel is being excluded from the displayed color value.

Suggested change
diameter = get_float32_le(bytes_list, 8)
diameter = get_float32_le(bytes_list, 8)
# The tag stores RGBA, but the alpha channel is always opaque / not meaningful for users,
# so we display only the RGB portion for readability by dropping the last byte.

Copilot uses AI. Check for mistakes.
...
```

- **Install keys on the Flipper:** two options
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

Inconsistent capitalization in list items. Change 'two options' to 'Two options' to match the capitalization style of other list items in this section.

Suggested change
- **Install keys on the Flipper:** two options
- **Install keys on the Flipper:** Two options

Copilot uses AI. Check for mistakes.
@queengooborg
Copy link
Owner

This PR seems to have a lot of changes which would be better to review individually. Could you please split this PR into smaller chunks?

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