Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for a new AT device type fsc_bw236, a WiFi module that has been verified on a demo board for WiFi connectivity, UDP communication with "link.rt-thread.org", and TCP server connections.
- Added device class constant
AT_DEVICE_CLASS_FSC_BW236to the AT device framework - Implemented complete socket operations including TCP/UDP client support, domain resolution, and network configuration
- Created sample code demonstrating device initialization and WiFi connection setup
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/at_sample_fsc_bw236.c | Sample code demonstrating how to register and configure the fsc_bw236 device with WiFi credentials |
| inc/at_device.h | Added device class constant for FSC_BW236 module (0X18U) |
| class/fsc-bw236/at_socket_fsc_bw236.c | Core socket operations implementation including connect, send, close, and domain resolution |
| class/fsc-bw236/at_device_fsc_bw236.h | Header file defining device structures, constants, and function prototypes |
| class/fsc-bw236/at_device_fsc_bw236.c | Device initialization, network configuration, and control operations implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #endif | ||
|
|
||
| static struct at_device_fsc_bw236 fsc0 = { | ||
| FSC_BW236_SAMPLE_DEIVCE_NAME, FSC_BW236_SAMPLE_CLIENT_NAME, FSC_BW236_SAMPLE_WIFI_SSID, |
There was a problem hiding this comment.
Inconsistent usage: The macro is defined as FSC_BW236_SAMPLE_DEIVCE_NAME (with typo) on line 14, but used here without the typo. After fixing the typo in the macro definition, this usage will be consistent.
| * create TCP/UDP or server connect by AT commands. | ||
| * | ||
| * @param socket current socket | ||
| * @param backlog waiting to handdle work, useless in "at mode" |
There was a problem hiding this comment.
Typo in comment: "handdle" should be "handle".
| * @param backlog waiting to handdle work, useless in "at mode" | |
| * @param backlog waiting to handle work, useless in "at mode" |
| uint8_t default_tcps_socket_num = 3; | ||
| uint8_t tcps_socket_num; | ||
| if ( type == AT_SOCKET_UDP ) { | ||
| tcps_socket_num = default_tcps_socket_num + 1; // MAX+1 | ||
| } | ||
|
|
||
| while ( sent_size < bfsz ) { | ||
| if ( bfsz - sent_size < FSC_BW236_MODULE_SEND_MAX_SIZE ) { | ||
| cur_pkt_size = bfsz - sent_size; | ||
| } | ||
| else { | ||
| cur_pkt_size = FSC_BW236_MODULE_SEND_MAX_SIZE; | ||
| } | ||
|
|
||
| uint8_t format_len = | ||
| rt_sprintf( ( char* )send_cmd, "AT+WFSEND=%d,%d,", tcps_socket_num, cur_pkt_size ); |
There was a problem hiding this comment.
Uninitialized variable usage: tcps_socket_num is only assigned when type == AT_SOCKET_UDP, but it's used unconditionally on line 281. For TCP sockets, this variable will be uninitialized. Either initialize it with a default value or handle TCP socket type as well.
| return fsc_bw236_netdev_set_down( device->netdev ); | ||
| } | ||
|
|
||
| /* reset eap8266 device and initialize device network again */ |
There was a problem hiding this comment.
Incorrect comment reference: This comment says "reset eap8266 device" but the function is for fsc_bw236 device. The comment should be updated to reference the correct device.
| /* reset eap8266 device and initialize device network again */ | |
| /* reset fsc_bw236 device and initialize device network again */ |
| return result; | ||
| } | ||
|
|
||
| /* change eap8266 wifi ssid and password information */ |
There was a problem hiding this comment.
Incorrect comment reference: This comment says "change eap8266 wifi ssid" but the function is for fsc_bw236 device. The comment should be updated to reference the correct device.
| /* change eap8266 wifi ssid and password information */ | |
| /* change fsc_bw236 wifi ssid and password information */ |
| * @brief calculates the decimal digits of an integer (ignoring the symbol) | ||
| * | ||
| * @param num integer value to calculate (negative numbers allowed) | ||
| * @return int Number digits (0 is counted as 1 digit, negative signs are not counted) | ||
| * | ||
| * @note Special value processing: | ||
| *-Enter 0 and return 1 | ||
| *-Negative numbers automatically take absolute values to calculate | ||
| * | ||
| * @code | ||
| *count_digits(-314) //Return 3 | ||
| *count_digits(0) //Return 1 | ||
| *count_digits(12345) //Return 5 |
There was a problem hiding this comment.
Typo in comment: "calculates" should be capitalized at the start of the sentence. Also, the comment formatting is inconsistent with English/Chinese mixing and has grammatical issues throughout lines 64-77.
| * @brief calculates the decimal digits of an integer (ignoring the symbol) | |
| * | |
| * @param num integer value to calculate (negative numbers allowed) | |
| * @return int Number digits (0 is counted as 1 digit, negative signs are not counted) | |
| * | |
| * @note Special value processing: | |
| *-Enter 0 and return 1 | |
| *-Negative numbers automatically take absolute values to calculate | |
| * | |
| * @code | |
| *count_digits(-314) //Return 3 | |
| *count_digits(0) //Return 1 | |
| *count_digits(12345) //Return 5 | |
| * @brief Calculates the number of decimal digits in an integer (ignoring the sign). | |
| * | |
| * @param num Integer value to calculate (negative numbers allowed). | |
| * @return int Number of digits (0 is counted as 1 digit; negative signs are not counted). | |
| * | |
| * @note Special cases: | |
| * - If the input is 0, returns 1. | |
| * - Negative numbers are converted to their absolute value before calculation. | |
| * | |
| * @code | |
| * count_digits(-314); // Returns 3 | |
| * count_digits(0); // Returns 1 | |
| * count_digits(12345); // Returns 5 |
| return FSC_BW236_GMR_AT_VERSION; | ||
| } | ||
|
|
||
| #endif /* AT_DEVICE_USING_fsc_bw236 */ |
There was a problem hiding this comment.
Incorrect comment conditional: The comment references "AT_DEVICE_USING_fsc_bw236" (lowercase) but the actual define used throughout the code is "AT_DEVICE_USING_FSC_BW236" (uppercase).
| #endif /* AT_DEVICE_USING_fsc_bw236 */ | |
| #endif /* AT_DEVICE_USING_FSC_BW236 */ |
| #define LOG_TAG "at.sample.fsc" | ||
| #include <at_log.h> | ||
|
|
||
| #define FSC_BW236_SAMPLE_DEIVCE_NAME "fsc0" |
There was a problem hiding this comment.
Typo in macro name: "DEIVCE" should be "DEVICE".
1 participant
增加AT设备类型fsc_bw236
已在demo板经过验证
可以正常连接wifi和使用UDP和"link.rt-thread.org"通信 也可以正常连接tcp服务器与之通信