XCSoar (.CUP) and Winpilot (.DAT) file processing now works #90
Conversation
There was a problem hiding this comment.
Congrats on figuring out the basic problem. :)
Can you please make a change that is as minimal as possible to fix the cup parsing issue and make a commit for it? No Test files or commented out debug code please, also no excessive logging which isn´t needed for operation.
|
Let me get my Linux laptop back out and I'll do the changes you requested ;) |
|
Regarding the .CUP file encoding issue, I checked both the Slovenia3.cup and the Slovenia3.cup, and it appears the Slovenia3.cup file is corrupted - at least in my Landscapes/Slovenia3 folder. I did see that Slovenia3.cup is encoded in UTF-8 as you stated, but for some reason the Slovenia2.cup file is encoded as 'ANSI'. In any case I'll change back to UTF-8, but what to do about the corrupted lines in Slovenia3.cup? Can you check Slovenia3.cup on your end? Here is my line for 'Amzs Koa' (which isn't in the Slovenia2.cup file) "Amzs Ko├Ã",Amzs Koì,4539.139N,01450.914E,470.0m,1,,,, |
|
commit 2abe1a4 (HEAD -> master) |
I also have condor3 and i see the same issue in the Slovenia3.cup file. So its really corrupted. |
|
Removed another couple of logging code lines, and added Slovenia3.cup/dat to .gtignore |
|
One comment line removed from server.py, Slovenia* removed. |
|
Haven't seen anything here recently. Do you have any other requested changes? |
|
Just checking in here. I see you have requested a change, but I can't see what the requested change is. Am I missing something? |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Warning Rate limit exceeded@lordfolken has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughThe changes focus on improving the readability, correctness, and robustness of waypoint file parsing and validation. They clarify file extension checks, consistently handle byte input lines by decoding them, refine header and comment skipping, and enhance type handling for coordinates and altitudes. No public interfaces or function signatures were altered. Changes
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Nitpick comments (3)
lib/xcsoar/mapgen/waypoints/seeyou_reader.py (1)
81-83: Remove unused line counter variable.The
wpnumvariable is incremented but never used elsewhere in the function. This appears to be leftover debugging code that should be removed.- wpnum = 0 for byteline in lines: - wpnum = wpnum + 1 line = byteline.decode("UTF-8")lib/xcsoar/mapgen/waypoints/parser.py (1)
12-12: Clarify the bugfix comment.The comment "# 241207 gfp bugfix:" is unclear. Please provide more context about what specific bug was fixed.
lib/xcsoar/mapgen/waypoints/winpilot_reader.py (1)
52-54: Consider utilizing the waypoint counter.A waypoint counter
wpnumis introduced but not used in the current implementation. If this is intended for future use (like line number reporting in error messages), consider adding a comment explaining its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)lib/xcsoar/mapgen/server/server.py(5 hunks)lib/xcsoar/mapgen/server/view.py(1 hunks)lib/xcsoar/mapgen/waypoints/list.py(1 hunks)lib/xcsoar/mapgen/waypoints/parser.py(1 hunks)lib/xcsoar/mapgen/waypoints/seeyou_reader.py(1 hunks)lib/xcsoar/mapgen/waypoints/winpilot_reader.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
lib/xcsoar/mapgen/waypoints/parser.py (2)
lib/xcsoar/mapgen/waypoints/winpilot_reader.py (1)
parse_winpilot_waypoints(49-73)lib/xcsoar/mapgen/waypoints/seeyou_reader.py (1)
parse_seeyou_waypoints(76-137)
lib/xcsoar/mapgen/server/server.py (4)
lib/xcsoar/mapgen/waypoints/parser.py (1)
parse_waypoint_file(6-16)lib/xcsoar/mapgen/waypoints/list.py (1)
get_bounds(33-43)lib/xcsoar/mapgen/server/view.py (1)
render(35-47)lib/xcsoar/mapgen/server/job.py (1)
error(65-70)
lib/xcsoar/mapgen/waypoints/winpilot_reader.py (1)
lib/xcsoar/mapgen/waypoints/list.py (1)
WaypointList(6-43)
⏰ 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: Codacy Static Code Analysis
🔇 Additional comments (13)
.gitignore (1)
18-19: LGTM! Proper cleanup of corrupted test files.Adding the corrupted Slovenia3.cup and Slovenia3.dat files to .gitignore is the correct approach based on the PR discussion about file corruption issues.
lib/xcsoar/mapgen/server/view.py (1)
44-44: LGTM! Good formatting improvement.The blank line improves code readability by providing visual separation before the Context creation.
lib/xcsoar/mapgen/waypoints/list.py (1)
42-42: LGTM! Addressed previous review feedback.The removal of the extraneous blank line addresses the previous review comment requesting this cleanup.
lib/xcsoar/mapgen/waypoints/seeyou_reader.py (2)
90-91: Good improvement for header detection.The explicit header checking with
if header in line:is a robust improvement that handles the updated CUP format standards mentioned in the past review comments.
97-100: Good variable naming improvement.Using
CSVlineinstead of reusinglineimproves code clarity and prevents confusion between the raw line and the CSV parsing object.lib/xcsoar/mapgen/waypoints/parser.py (1)
10-12: Good alignment with parsing function expectations.Passing the lines list to the parsing functions is correct since both
parse_winpilot_waypointsandparse_seeyou_waypointsexpect a list of byte lines to decode, as shown in the relevant code snippets.lib/xcsoar/mapgen/server/server.py (3)
65-65: Minor formatting improvements.The added whitespace improves code readability and consistency.
Also applies to: 79-79, 88-88, 111-111, 153-153
89-103: Excellent fix to the file extension validation logic.The previous logic had a confusing condition that would incorrectly reject valid files. The new implementation clearly validates that the file must have either a
.dator.cupextension, which aligns with the supported formats mentioned in the PR objectives.
112-114: Temporary bounds display solution works as intended.This implementation addresses the PR objective where automatic display of bounds in the web interface wasn't fully implemented. Showing the calculated bounds in the error message allows users to manually copy these values as mentioned in the PR summary.
lib/xcsoar/mapgen/waypoints/winpilot_reader.py (4)
13-16: Improved altitude parsing to handle decimal values.The change from direct
int(str)toint(float(str))properly handles altitude strings that may contain decimal values, preventing ValueError exceptions.
18-24: Helpful documentation for coordinate formats.The added comments clearly document the expected coordinate formats for Winpilot .DAT files, improving code maintainability.
31-31: Good practice: avoid variable name shadowing.Renaming the variable from
strtostrsplitprevents shadowing of the built-instrtype and makes the code more readable. All references have been correctly updated.Also applies to: 32-32, 35-35, 37-37, 39-39
53-56: UTF-8 decoding implementation aligns with parser refactoring.The change to iterate over byte lines and decode UTF-8 is consistent with the parser.py refactoring mentioned in the relevant snippets. This addresses the encoding issues discussed in the PR comments.
|
@coderabbitai please create a commit that removes the addtions to .gitignore |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@lordfolken, I'll help you remove the additions to .gitignore. Let me first examine the current state of the file to see exactly what needs to be removed. 🏁 Script executed: Length of output: 158 I can see the current .gitignore file and the two additions that need to be removed. Let me plan the code changes to remove Slovenia3.cup and Slovenia3.dat from the .gitignore file. Task: Remove Slovenia3.cup and Slovenia3.dat from .gitignore
The plan is ready! Another agent will now execute these changes and create a commit that removes the Slovenia3.cup and Slovenia3.dat entries from the .gitignore file. ⏳ Preparing changes... |
|
A summary of the changes CodeRabbit can apply:
|
|
Caution CodeRabbit has failed to create a local commit with your edits. |
1 similar comment
|
Caution CodeRabbit has failed to create a local commit with your edits. |
lordfolken
force-pushed
the
master
branch
3 times, most recently
from
859e1a4 to
e0bb35b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/xcsoar/mapgen/waypoints/seeyou_reader.py (1)
84-90: Improve exception chaining for better error context.The UTF-8 decoding error handling correctly prevents data loss as required for aviation safety, but the exception chaining could be improved.
Apply this diff to follow Python best practices for exception chaining:
except UnicodeDecodeError as e: raise RuntimeError( f"Failed to decode line {wpnum} as UTF-8: {e}. " f"Please check the waypoint file encoding and fix any corrupted data." - ) + ) from eThis preserves the original exception context while providing the enhanced error message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/xcsoar/mapgen/server/server.py(2 hunks)lib/xcsoar/mapgen/server/view.py(1 hunks)lib/xcsoar/mapgen/waypoints/list.py(1 hunks)lib/xcsoar/mapgen/waypoints/parser.py(1 hunks)lib/xcsoar/mapgen/waypoints/seeyou_reader.py(1 hunks)lib/xcsoar/mapgen/waypoints/winpilot_reader.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/xcsoar/mapgen/server/view.py
- lib/xcsoar/mapgen/waypoints/list.py
- lib/xcsoar/mapgen/server/server.py
- lib/xcsoar/mapgen/waypoints/parser.py
- lib/xcsoar/mapgen/waypoints/winpilot_reader.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lordfolken
PR: XCSoar/mapgen#90
File: lib/xcsoar/mapgen/waypoints/seeyou_reader.py:84-84
Timestamp: 2025-07-16T10:29:46.065Z
Learning: In aviation contexts, waypoint data integrity is critical for navigation safety. Error handling should never silently skip or lose waypoint data as this could result in missing navigation points that pilots need. Always fail explicitly with clear error messages rather than silently losing data.
lib/xcsoar/mapgen/waypoints/seeyou_reader.py (1)
Learnt from: lordfolken
PR: XCSoar/mapgen#90
File: lib/xcsoar/mapgen/waypoints/seeyou_reader.py:84-84
Timestamp: 2025-07-16T10:29:46.065Z
Learning: In aviation contexts, waypoint data integrity is critical for navigation safety. Error handling should never silently skip or lose waypoint data as this could result in missing navigation points that pilots need. Always fail explicitly with clear error messages rather than silently losing data.
🪛 Ruff (0.12.2)
lib/xcsoar/mapgen/waypoints/seeyou_reader.py
87-90: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
⏰ 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: Codacy Static Code Analysis
🔇 Additional comments (4)
lib/xcsoar/mapgen/waypoints/seeyou_reader.py (4)
79-80: LGTM: Header definition improves robustness.The explicit header definition makes the code more maintainable and self-documenting compared to relying on positional assumptions.
81-83: LGTM: Line counter enhances error reporting.The line counter provides valuable context for error messages, which is crucial for aviation applications where data integrity issues must be precisely identified.
96-97: LGTM: Header detection replaces fragile line skipping.The header detection is more robust than unconditionally skipping the first line, as it correctly handles files with different structures or missing headers.
103-106: LGTM: Variable naming improves code clarity.Using
CSVlineinstead of reusinglinemakes the code more readable and reduces potential confusion between the raw line and the CSV parser object.
|
@coderabbitai hey bunny, can you analyze server.py and see why the bounds would not get parsed when uploading a foobar.cup file? |
|
Tip For best results, initiate chat on the files or code changes. A summary of the changes CodeRabbit can apply:
|
|
Caution CodeRabbit has failed to create a local commit with your edits. |
2 participants
XCSoar (.CUP) and Winpilot (.DAT) file processing now works properly. Unfortunately I couldn't figure out how to get the left/right/top/bottom bounds values to show up in the corresponding web page windows, so the map still isn't generated automatically.
As a workaround the calculated bounds are displayed in the 'error' window, and the user can manually transcribe the values to the bounds entry fields and then select the 'select map bounds by hand' option.
Summary by CodeRabbit
Bug Fixes
Refactor