Skip to content

app: Update socket AT commands to use handle-based API#26

Merged
juhaylinen merged 1 commit into
nrfconnect:mainfrom
juhaylinen:add_socket_id
Oct 17, 2025
Merged

app: Update socket AT commands to use handle-based API#26
juhaylinen merged 1 commit into
nrfconnect:mainfrom
juhaylinen:add_socket_id

Conversation

@juhaylinen
Copy link
Copy Markdown
Contributor

All socket operations now require a handle obtained from #XSOCKET or #XSSOCKET command. This allows more flexible way to manage sockets.

Remove AT command #XSOCKETSELECT.
Add AT command #XCLOSE to close individual socket or all sockets at once.

Update documentation.

SLM-50

Copilot AI review requested due to automatic review settings October 13, 2025 12:21
Copy link
Copy Markdown

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 updates socket AT commands to use a handle-based API instead of socket selection, providing more flexible socket management by directly referencing socket handles in all operations.

  • Removes AT#XSOCKETSELECT command and socket ranking system
  • Adds AT#XCLOSE command for individual or bulk socket closure
  • Updates all socket operations to require handle parameters

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
doc/app/sm_migration.rst Adds migration guide documenting the transition from socket selection to handle-based API
doc/app/SOCKET_AT_commands.rst Updates command documentation to reflect handle-based parameters and new #XCLOSE command
app/src/sm_at_socket.c Implements handle-based socket operations, removes socket selection logic, and adds close command handler

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c
@juhaylinen juhaylinen force-pushed the add_socket_id branch 2 times, most recently from 24b8903 to f575160 Compare October 13, 2025 13:30
Copy link
Copy Markdown
Contributor

@MarkusLassila MarkusLassila left a comment

Choose a reason for hiding this comment

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

Moving into good direction!

I'll review the documentation after the sm_at_socket.c is in order.

Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c
Comment thread app/src/sm_at_socket.c
Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c
Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c Outdated
Comment thread app/src/sm_at_socket.c
Comment thread doc/migration_notes.rst Outdated
@juhaylinen juhaylinen force-pushed the add_socket_id branch 3 times, most recently from 894825a to 17e88a2 Compare October 16, 2025 10:46
@juhaylinen
Copy link
Copy Markdown
Contributor Author

@MarkusLassila @trantanen I addressed the findings. Please re-review.

Copy link
Copy Markdown
Contributor

@MarkusLassila MarkusLassila left a comment

Choose a reason for hiding this comment

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

Some nits, but LGTM!

Comment thread app/src/sm_at_socket.c
Comment thread doc/app/SOCKET_AT_commands.rst Outdated
Comment thread doc/app/SOCKET_AT_commands.rst
Comment thread doc/app/SOCKET_AT_commands.rst
Comment thread doc/app/SOCKET_AT_commands.rst
Comment thread doc/migration_notes.rst Outdated
@juhaylinen juhaylinen force-pushed the add_socket_id branch 2 times, most recently from 0e4271b to 1c6e9ff Compare October 16, 2025 13:22
Comment thread app/src/sm_at_socket.c Outdated

util_get_peer_addr(&remote, peer_addr, &peer_port);
rsp_send("\r\n#XRECVFROM: %d,\"%s\",%d\r\n", ret, peer_addr, peer_port);
rsp_send("\r\n#XRECVFROM: %d,%d,\"%s\",%d\r\n", sock->fd, ret, peer_addr, peer_port);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@juhaylinen: Handle these github actions complaints.

Copy link
Copy Markdown
Collaborator

@trantanen trantanen Oct 17, 2025

Choose a reason for hiding this comment

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

Indeed, compliance has several warnings about long lines but the check is showing green in github. Compliance log has these long lines as warnings, not errors so it doesn't fail. I will need to look at this but meanwhile, please fix the failures.

Comment thread doc/migration_notes.rst Outdated
Comment thread app/src/sm_at_socket.c
All socket operations now require a handle obtained from #XSOCKET
or #XSSOCKET command. This allows more flexible way to manage
sockets.

Remove AT command #XSOCKETSELECT.
Add AT command #XCLOSE to close individual socket or all sockets
at once.

Update documentation.

SLM-50

Signed-off-by: Juha Ylinen <juha.ylinen@nordicsemi.no>
Copy link
Copy Markdown
Contributor

@MarkusLassila MarkusLassila left a comment

Choose a reason for hiding this comment

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

Merge away.

@moksanen && @jvaliharju, prepare for breaking socket tests.

@juhaylinen juhaylinen merged commit 26429a7 into nrfconnect:main Oct 17, 2025
2 checks passed
@juhaylinen juhaylinen deleted the add_socket_id branch October 17, 2025 10:16
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.

4 participants