feat: 添加JTAG 20P兼容模式 #50
Conversation
WalkthroughThe pull request refactors JSON boolean retrieval by removing a lambda and replacing it with a generic Changes
Sequence Diagram(s)sequenceDiagram
participant S as update_settings()
participant U as uartx_io_init()
participant C as UART_Config
S->>U: Call uartx_io_init() after updating settings
alt jtag_20pin_compatible == false
U->>C: Configure pins for UART mode
else jtag_20pin_compatible == true
U->>C: Configure pins for GPIO mode with specific settings
end
Possibly related PRs
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:
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: 0
🧹 Nitpick comments (3)
projects/HSLink-Pro/src/setting.cpp (3)
90-94: Consider adding return typeThe function declarations are missing return types. While C++ allows this, it's better to be explicit for readability and consistency with the rest of the codebase.
-static void store_settings(); -static void load_settings(); -static void update_settings(); +static void store_settings(void); +static void load_settings(void); +static void update_settings(void);
33-45: Initialize the new jtag_20pin_compatible fieldThe
jtag_20pin_compatiblefield is not initialized in theHSLink_Settingstruct. While default values are provided when reading from JSON, it's good practice to initialize all fields.HSLink_Setting_t HSLink_Setting = { .boost = false, .swd_port_mode = PORT_MODE_SPI, .jtag_port_mode = PORT_MODE_SPI, .power = { .vref = 3.3, .power_on = false, .port_on = false, }, .reset = 0, .led = false, .led_brightness = 0, + .jtag_20pin_compatible = false, };
19-31: Initialize the new jtag_20pin_compatible field in default_settingThe
jtag_20pin_compatiblefield is not initialized in thedefault_settingconstant. While default values are provided when reading from JSON, it's good practice to initialize all fields.const HSLink_Setting_t default_setting = { .boost = false, .swd_port_mode = PORT_MODE_SPI, .jtag_port_mode = PORT_MODE_SPI, .power = { .vref = 3.3, .power_on = true, .port_on = true, }, .reset = 1 << RESET_NRST, .led = true, .led_brightness = 10, + .jtag_20pin_compatible = false, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
projects/HSLink-Pro/src/HID_COMM/hid_comm.cpp(3 hunks)projects/HSLink-Pro/src/USB2UART/usb2uart.c(8 hunks)projects/HSLink-Pro/src/USB2UART/usb2uart.h(1 hunks)projects/HSLink-Pro/src/setting.cpp(6 hunks)projects/HSLink-Pro/src/setting.h(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
projects/HSLink-Pro/src/setting.cpp (2)
projects/HSLink-Pro/src/USB2UART/usb2uart.h (1)
uartx_io_init(12-12)projects/HSLink-Pro/src/USB2UART/usb2uart.c (1)
uartx_io_init(121-169)
projects/HSLink-Pro/src/USB2UART/usb2uart.c (4)
projects/HSLink-Pro/src/USB2UART/usb2uart.h (3)
usb2uart_handler(16-16)uartx_io_init(12-12)uartx_preinit(14-14)projects/HSLink-Pro/common/hslinkpro/board.h (1)
CheckHardwareVersion(48-48)projects/HSLink-Pro/common/hslinkpro/board.c (1)
CheckHardwareVersion(182-194)dap_main.h (2)
chry_dap_usb2uart_uart_config_callback(135-135)chry_dap_usb2uart_uart_send_bydma(137-137)
🔇 Additional comments (17)
projects/HSLink-Pro/src/USB2UART/usb2uart.h (1)
12-12: LGTM: Function declaration for UART I/O initializationThe function declaration for
uartx_io_init()looks good, providing a dedicated method for UART pin configuration.projects/HSLink-Pro/src/HID_COMM/hid_comm.cpp (3)
237-237: LGTM: Using the new template functionGood refactoring to use the generic
get_json_valuefunction instead of a lambda.
258-259: LGTM: Proper implementation of the new settingCorrectly implements the new
jtag_20pin_compatiblesetting with a default value offalse, which is a safe default for backward compatibility.
381-383: LGTM: Complete JSON responseProperly includes the new
jtag_20pin_compatiblesetting in the JSON response.projects/HSLink-Pro/src/setting.h (2)
40-40: LGTM: New setting field addedThe
jtag_20pin_compatiblefield is correctly added to theHSLink_Setting_tstructure.
50-70: LGTM: Well-designed template specializationsGood implementation of template specializations for different data types. These functions handle the case where the key doesn't exist in the JSON by returning a default value.
projects/HSLink-Pro/src/setting.cpp (6)
15-15: LGTM: Added the necessary includeCorrectly includes "usb2uart.h" to access the
uartx_io_initfunction.
50-88: LGTM: Template specialization implementationsGood implementation of the template specializations to handle various data types for JSON value retrieval.
147-149: LGTM: JSON output updatedCorrectly adds the
jtag_20pin_compatiblefield to the JSON output.
169-169: LGTM: Using templated functionGood use of the generic
get_json_valuefunction.
189-189: LGTM: Setting the new field from JSONCorrectly sets the
jtag_20pin_compatiblefield from the JSON input.
219-219: LGTM: UART initializationGood addition of
uartx_io_init()call to update UART pins when settings change.projects/HSLink-Pro/src/USB2UART/usb2uart.c (5)
171-174: Good refactoring of initialization logicThe modified
uartx_preinitfunction now delegates pin configuration to the dedicateduartx_io_initfunction, which improves code modularity and maintainability.
287-293: DTR control properly respects JTAG compatibility modeThe conditional check ensures that DTR signals are only modified when not in JTAG 20pin compatible mode, which correctly preserves the pin state for JTAG functionality when needed.
296-302: RTS control properly respects JTAG compatibility modeThe conditional check ensures that RTS signals are only modified when not in JTAG 20pin compatible mode, which correctly preserves the pin state for JTAG functionality when needed.
42-43: Consistent formatting changes across function definitionsThe opening braces for multiple function definitions have been moved to a new line, maintaining consistent coding style throughout the file.
Also applies to: 65-66, 91-92, 184-185, 207-208, 221-222
121-169:⚠️ Potential issueWell-structured implementation of UART pin configuration logic
The new
uartx_io_initfunction effectively implements the JTAG 20pin compatibility mode by conditionally configuring UART pins. When this mode is enabled, the UART pins are set to GPIO mode with appropriate default levels.However, there's a syntax error on line 137:
- HPM_IOC->PAD[PIN_UART_RX].FUNC_CTL = IOC_PAD_FUNC_CTL_ALT_SELECT_SET(0);; + HPM_IOC->PAD[PIN_UART_RX].FUNC_CTL = IOC_PAD_FUNC_CTL_ALT_SELECT_SET(0);Likely an incorrect or invalid review comment.
|
在从db的setting json拿配置字段的时候最好也做一下判断？ |
Summary by CodeRabbit
New Features
Refactor
Style