adding checksum #12
adding checksum #12
Conversation
WalkthroughMethod signatures were updated to accept an ST file path. The debugger generation now computes an MD5 of the ST file, embeds it in the returned C debugger string, and returns (cfile, c_debug). Call sites were adjusted to pass the ST path and use the updated return. A comment was removed. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as xml2st.main()
participant Gen as generate_debugger_file(csv_file, st_file)
participant PC as ProjectController.Generate_embedded_plc_debugger(st_file)
participant FS as Filesystem
CLI->>Gen: generate_debugger_file(csv_file, st_file)
Gen->>PC: Generate_embedded_plc_debugger(st_file)
PC->>FS: Read ST file and compute MD5
FS-->>PC: MD5 hash
PC-->>Gen: (cfile, c_debug with MD5 prefix)
Gen-->>CLI: Debugger text (c_debug) and outputs as needed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
xml2st.py (1)
78-85: Fix inconsistent return type and validate st_file in generate_debugger_fileWhen the CSV is invalid, this returns a tuple (None, None) but callers expect a string. This will crash downstream on debug_text.split(...). Also add a basic st_file validation here (generate_debugger_file is public and may be called without the prior parse step).
Apply:
def generate_debugger_file(csv_file, st_file): if not os.path.isfile(csv_file) or not csv_file.lower().endswith(".csv"): print( f"Error: Invalid file '{csv_file}'. A path to a csv file is expected.", file=sys.stderr, ) - return None, None + return None + + # Validate the ST file as well + if not os.path.isfile(st_file) or not st_file.lower().endswith(".st"): + print( + f"Error: Invalid file '{st_file}'. A path to a st file is expected.", + file=sys.stderr, + ) + return None
🧹 Nitpick comments (2)
xml2st.py (1)
88-88: Consider preserving both outputs from Generate_embedded_plc_debuggerYou’re discarding cfile. Returning both (cfile, c_debug) from generate_debugger_file would let CLI/reporting surface where the C file was written without another change later. If you keep returning only c_debug, consider documenting that or renaming for clarity.
- return controler.Generate_embedded_plc_debugger(st_file)[1] + # Option A (recommended): return both for callers that need the path + # return controler.Generate_embedded_plc_debugger(st_file) + # Option B: keep returning just the C debug payload + return controler.Generate_embedded_plc_debugger(st_file)[1]ProjectController.py (1)
179-186: Robustify MD5 computation, remove dead check, and fix invalid raise
- Use a context manager and stream hashing (SIM115).
- hashlib.md5(...).hexdigest() never returns None; the check is dead code.
- raise ("string") is invalid in Python 3 (B016).
- Minor: make the C symbol less collision-prone and use f-strings.
- # Wrap debugger code around (* comments *) - MD5 = hashlib.md5(open(st_file, "rb").read()).hexdigest() - if MD5 is None: - raise ("Error building project: md5 object is null\n") - - # Add MD5 value to debug.cpp file - c_debug = 'char md5[] = "' + MD5 + '";\n' + debug_text + # Compute MD5 of the ST file for provenance + hasher = hashlib.md5() + with open(st_file, "rb") as f: + for chunk in iter(lambda: f.read(8192), b""): + hasher.update(chunk) + md5_hex = hasher.hexdigest() + + # Prefix the debug C text with the checksum as a C string + c_debug = f'static const char debug_md5[] = "{md5_hex}";\n{debug_text}'If you do want explicit error handling on missing paths, wrap the open() in try/except and raise a proper Exception type with context (e.g., FileNotFoundError or a RuntimeError with the path).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
ProjectController.py(3 hunks)xml2st.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
xml2st.py (1)
ProjectController.py (1)
Generate_embedded_plc_debugger(162-187)
🪛 Ruff (0.12.2)
ProjectController.py
180-180: Use a context manager for opening files
(SIM115)
182-182: Cannot raise a literal. Did you intend to return it or raise an Exception?
(B016)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-windows-arm
🔇 Additional comments (2)
ProjectController.py (2)
5-5: LGTM: hashlib import is correct and scoped where usedThis directly supports the new checksum feature and introduces no side effects.
162-162: All Generate_embedded_plc_debugger call sites updated
- Verified via
rgthat the only invocation is in xml2st.py (line 88), passingst_fileas the sole argument- No other occurrences of the old signature remain
thiagoralves
requested review from
thiagoralves
and removed request for
thiagoralves
2 participants
Summary by CodeRabbit