Skip to content
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

fix(modem): Fixed AT commands to copy strings to prevent overrides #472

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -25,18 +25,16 @@ command_result generic_command(CommandableIf *t, const std::string &command,
* @brief Utility command to send command and return reply (after DCE says OK)
* @param t Anything that is "command-able"
* @param command Command to issue
* @param output String to return
* @param timeout_ms
* @param output String to return (could be either std::string& or std::string_view&)
* @param timeout_ms Command timeout in ms
* @return Generic command return type (OK, FAIL, TIMEOUT)
*/
command_result generic_get_string(CommandableIf *t, const std::string &command, std::string &output, uint32_t timeout_ms = 500);
template <typename T> command_result generic_get_string(CommandableIf *t, const std::string &command, T &output, uint32_t timeout_ms = 500);

/**
* @brief Generic command that passes on "OK" and fails on "ERROR"
* @param t Anything that is "command-able"
* @param command Command to issue
* @param timeout
* @param timeout_ms Command timeout in ms
* @return Generic command return type (OK, FAIL, TIMEOUT)
*/
Expand Down
64 changes: 41 additions & 23 deletions components/esp_modem/src/esp_modem_command_library.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,35 @@ command_result generic_command(CommandableIf *t, const std::string &command,
return generic_command(t, command, pass, fail, timeout_ms);
}

static command_result generic_get_string(CommandableIf *t, const std::string &command, std::string_view &output, uint32_t timeout_ms = 500)
/*
* Purpose of this namespace is to provide different means of assigning the result to a string-like parameter.
* By default we assign strings, which comes with an allocation. Alternatively we could take `std::span`
* with user's buffer and directly copy the result, thus avoiding allocations (this is not used as of now)
*/
namespace str_copy {

bool set(std::string &dest, std::string_view &src)
{
dest = src;
return true;
}

/* This is an example of using std::span output in generic_get_string()
bool set(std::span<char> &dest, std::string_view &src)
{
if (dest.size() >= src.size()) {
std::copy(src.begin(), src.end(), dest.data());
dest = dest.subspan(0, src.size());
return true;
}
ESP_LOGE(TAG, "Cannot set result of size %d (to span of size %d)", dest.size(), src.size());
return false;
}
*/

} // str_copy

template <typename T> command_result generic_get_string(CommandableIf *t, const std::string &command, T &output, uint32_t timeout_ms)
{
ESP_LOGV(TAG, "%s", __func__ );
return t->command(command, [&](uint8_t *data, size_t len) {
Expand All @@ -70,26 +98,16 @@ static command_result generic_get_string(CommandableIf *t, const std::string &co
} else if (token.find("ERROR") != std::string::npos) {
return command_result::FAIL;
} else if (token.size() > 2) {
output = token;
if (!str_copy::set(output, token)) {
return command_result::FAIL;
}
}
response = response.substr(pos + 1);
}
return command_result::TIMEOUT;
}, timeout_ms);
}

command_result generic_get_string(CommandableIf *t, const std::string &command, std::string &output, uint32_t timeout_ms)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
auto ret = generic_get_string(t, command, out, timeout_ms);
if (ret == command_result::OK) {
output = out;
}
return ret;
}


command_result generic_command_common(CommandableIf *t, const std::string &command, uint32_t timeout_ms)
{
ESP_LOGV(TAG, "%s", __func__ );
Expand Down Expand Up @@ -153,7 +171,7 @@ command_result hang_up(CommandableIf *t)
command_result get_battery_status(CommandableIf *t, int &voltage, int &bcs, int &bcl)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CBC\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -189,7 +207,7 @@ command_result get_battery_status(CommandableIf *t, int &voltage, int &bcs, int
command_result get_battery_status_sim7xxx(CommandableIf *t, int &voltage, int &bcs, int &bcl)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CBC\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -224,7 +242,7 @@ command_result set_flow_control(CommandableIf *t, int dce_flow, int dte_flow)
command_result get_operator_name(CommandableIf *t, std::string &operator_name, int &act)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+COPS?\r", out, 75000);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -361,7 +379,7 @@ command_result set_cmux(CommandableIf *t)
command_result read_pin(CommandableIf *t, bool &pin_ok)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CPIN?\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -413,7 +431,7 @@ command_result at_raw(CommandableIf *t, const std::string &cmd, std::string &out
command_result get_signal_quality(CommandableIf *t, int &rssi, int &ber)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CSQ\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -451,7 +469,7 @@ command_result set_network_attachment_state(CommandableIf *t, int state)
command_result get_network_attachment_state(CommandableIf *t, int &state)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CGATT?\r", out);
if (ret != command_result::OK) {
return ret;
Expand All @@ -478,7 +496,7 @@ command_result set_radio_state(CommandableIf *t, int state)
command_result get_radio_state(CommandableIf *t, int &state)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CFUN?\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -543,7 +561,7 @@ command_result set_network_bands_sim76xx(CommandableIf *t, const std::string &mo
command_result get_network_system_mode(CommandableIf *t, int &mode)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CNSMOD?\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down Expand Up @@ -571,7 +589,7 @@ command_result set_gnss_power_mode(CommandableIf *t, int mode)
command_result get_gnss_power_mode(CommandableIf *t, int &mode)
{
ESP_LOGV(TAG, "%s", __func__ );
std::string_view out;
std::string out;
auto ret = generic_get_string(t, "AT+CGNSPWR?\r", out);
if (ret != command_result::OK) {
return ret;
Expand Down