-
Notifications
You must be signed in to change notification settings - Fork 16
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
[develop] Add basic support for ED64 X series #137
base: develop
Are you sure you want to change the base?
[develop] Add basic support for ED64 X series #137
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce a new source file for the ED64 flashcart implementation and modify existing files to incorporate support for a new feature, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Flashcart
participant ED64X
User->>Flashcart: Initialize ED64X
Flashcart->>ED64X: Call ed64x_get_flashcart()
ED64X-->>Flashcart: Return ED64X instance
Flashcart-->>User: ED64X Initialized
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (5)
src/flashcart/ed64x/ed64x.c (4)
26-29
: Initialization function is a placeholder.The
ed64x_init
function currently does nothing and returnsFLASHCART_OK
. Ensure to implement any necessary initialization logic in the future.
31-34
: De-initialization function is a placeholder.The
ed64x_deinit
function currently does nothing and returnsFLASHCART_OK
. Ensure to implement any necessary de-initialization logic in the future.
132-136
: Save loading function is a placeholder.The
ed64x_load_save
function currently does nothing and returnsFLASHCART_OK
. Ensure to implement the save loading logic in the future.
138-142
: Set save type function is a placeholder.The
ed64x_set_save_type
function currently does nothing and returnsFLASHCART_OK
. Ensure to implement the logic for setting save types in the future.src/flashcart/flashcart.c (1)
116-116
: Clarify comment forCART_ED
.The updated comment for
CART_ED
as "Series V EverDrive-64 or clone" provides better context and clarity.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- Makefile (1 hunks)
- src/flashcart/64drive/64drive.c (1 hunks)
- src/flashcart/ed64x/ed64x.c (1 hunks)
- src/flashcart/ed64x/ed64x.h (1 hunks)
- src/flashcart/ed64x/ed64x_ll.h (1 hunks)
- src/flashcart/flashcart.c (2 hunks)
- src/flashcart/flashcart.h (1 hunks)
- src/flashcart/sc64/sc64.c (1 hunks)
Files skipped from review due to trivial changes (1)
- src/flashcart/ed64x/ed64x_ll.h
Additional comments not posted (21)
src/flashcart/ed64x/ed64x.h (1)
1-24
: Header file looks good.The header file is well-structured with appropriate header guards and a clear function declaration for
ed64x_get_flashcart
.src/flashcart/flashcart.h (1)
32-32
: Feature addition looks good.The addition of
FLASHCART_FEATURE_AUTO_CIC
to theflashcart_features_t
enumeration is consistent with the existing features.src/flashcart/ed64x/ed64x.c (3)
41-50
: Feature checking relies on incomplete function.The
ed64x_has_feature
function logic appears correct but depends on the incompleteget_cart_model
function. Ensure thatget_cart_model
is implemented correctly for accurate feature checking.
52-92
: ROM loading function looks good.The
ed64x_load_rom
function is well-implemented, with appropriate error handling and progress reporting.
94-130
: File loading function looks good.The
ed64x_load_file
function is well-implemented, with appropriate error handling.Makefile (1)
33-33
: LGTM! Addition ofed64x.c
is appropriate.The inclusion of
flashcart/ed64x/ed64x.c
in theSRCS
variable is correct and aligns with the PR objective of adding support for the ED64 X series.src/flashcart/flashcart.c (2)
14-14
: Includeed64x.h
for ED64 X series support.The inclusion of
ed64x.h
is necessary for the integration of the ED64 X series functionality.
112-113
: Update forCART_EDX
initialization is appropriate.The change to use
ed64x_get_flashcart()
forCART_EDX
aligns with the new support for the Official EverDrive 64 Series X.src/flashcart/64drive/64drive.c (1)
78-78
: Support forFLASHCART_FEATURE_AUTO_CIC
added.The addition of the
FLASHCART_FEATURE_AUTO_CIC
case ind64_has_feature
expands the function's capability to recognize this feature.src/flashcart/sc64/sc64.c (12)
256-258
: Addition ofFLASHCART_FEATURE_AUTO_CIC
looks good!The new feature case has been correctly added to the
sc64_has_feature
function.
Line range hint
72-94
: Functionload_to_flash
is well-implemented.The function handles loading data to flash memory with appropriate error handling and progress updates.
Line range hint
97-103
: Functiondisk_zone_track_is_bad
is correctly implemented.The logic for checking bad tracks is straightforward and correct.
Line range hint
105-111
: Functiondisk_system_lba_is_bad
is correctly implemented.The logic for checking bad LBAs is straightforward and correct.
Line range hint
113-118
: Functiondisk_set_thb_mapping
is correctly implemented.The logic for setting THB mapping is correct and follows the expected pattern.
Line range hint
120-154
: Functiondisk_load_thb_table
is correctly implemented.The logic for loading the THB table is sound and follows the expected pattern.
Line range hint
156-164
: Functiondisk_load_sector_table
is correctly implemented.The logic for loading the sector table is sound and follows the expected pattern.
Line range hint
167-205
: Functionsc64_init
is correctly implemented.The logic for initializing the SC64 flashcart is sound and follows the expected pattern.
Line range hint
207-212
: Functionsc64_deinit
is correctly implemented.The logic for deinitializing the SC64 flashcart is sound and follows the expected pattern.
Line range hint
260-317
: Functionsc64_load_rom
is correctly implemented.The logic for loading a ROM file is sound and follows the expected pattern.
Line range hint
319-342
: Functionsc64_load_file
is correctly implemented.The logic for loading a file into memory is sound and follows the expected pattern.
Line range hint
344-381
: Functionsc64_load_save
is correctly implemented.The logic for loading a save file is sound and follows the expected pattern.
src/flashcart/ed64x/ed64x.c
Outdated
static ed64x_device_variant_t get_cart_model() { | ||
// Currently either X5 or X7 | ||
return true; // FIXME: check cart model. | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete function implementation.
The get_cart_model
function is incomplete and contains a FIXME comment. It currently returns true
, which is likely incorrect. Implement the logic to determine the actual cart model.
c094149
to
f7472bb
Compare
Only loads ROM's
towards get_cart_model.
Add feature improvements
31d0758
to
866387a
Compare
Improve variant enumeration
866387a
to
8226d0e
Compare
Only loads ROM's
towards get_cart_model.
Add feature improvements
Improve variant enumeration
Change the first num to indicate the bootloader.
This change is dependent on libcart changes for detection to work properly:
We await the change to be merged to libdragon. |
6990621
to
fb0a04e
Compare
Description
Add basic support for ED64 X series
Only loads ROM's (no save support)
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist:
Signed-off-by: GITHUB_USER <GITHUB_USER_EMAIL>
Summary by CodeRabbit
New Features
Bug Fixes
Documentation