Skip to content

Conversation

@illustriousness
Copy link

增加AT设备类型fsc_bw236
已在demo板经过验证
可以正常连接wifi和使用UDP和"link.rt-thread.org"通信 也可以正常连接tcp服务器与之通信

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_BW236 to 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,
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
* create TCP/UDP or server connect by AT commands.
*
* @param socket current socket
* @param backlog waiting to handdle work, useless in "at mode"
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment: "handdle" should be "handle".

Suggested change
* @param backlog waiting to handdle work, useless in "at mode"
* @param backlog waiting to handle work, useless in "at mode"

Copilot uses AI. Check for mistakes.
Comment on lines +266 to +281
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 );
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
return fsc_bw236_netdev_set_down( device->netdev );
}

/* reset eap8266 device and initialize device network again */
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
/* reset eap8266 device and initialize device network again */
/* reset fsc_bw236 device and initialize device network again */

Copilot uses AI. Check for mistakes.
return result;
}

/* change eap8266 wifi ssid and password information */
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
/* change eap8266 wifi ssid and password information */
/* change fsc_bw236 wifi ssid and password information */

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +76
* @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
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
* @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

Copilot uses AI. Check for mistakes.
return FSC_BW236_GMR_AT_VERSION;
}

#endif /* AT_DEVICE_USING_fsc_bw236 */
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
#endif /* AT_DEVICE_USING_fsc_bw236 */
#endif /* AT_DEVICE_USING_FSC_BW236 */

Copilot uses AI. Check for mistakes.
#define LOG_TAG "at.sample.fsc"
#include <at_log.h>

#define FSC_BW236_SAMPLE_DEIVCE_NAME "fsc0"
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in macro name: "DEIVCE" should be "DEVICE".

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant