Address review feedback: keep flush ownership in SerialProxy, per-pin modem checks, zwave NOT_SUPPORTED status

This commit is contained in:
kbx81
2026-08-15 01:23:50 -05:00
parent 8c9e11d923
commit 98dce7949b
7 changed files with 54 additions and 47 deletions
+3 -2
View File
@@ -2623,8 +2623,9 @@ message ZWaveProxyRequest {
}
enum ZWaveProxyStatus {
ZWAVE_PROXY_STATUS_OK = 0; // Request completed successfully
ZWAVE_PROXY_STATUS_IN_USE = 1; // Denied: another client is already subscribed
ZWAVE_PROXY_STATUS_OK = 0; // Request completed successfully
ZWAVE_PROXY_STATUS_IN_USE = 1; // Denied: another client is already subscribed
ZWAVE_PROXY_STATUS_NOT_SUPPORTED = 2; // Request type not supported
}
// Acknowledges a ZWaveProxyRequest (subscribe/unsubscribe). Sent since API 1.16.
+10 -30
View File
@@ -1384,16 +1384,11 @@ void APIConnection::on_z_wave_proxy_frame(const ZWaveProxyFrame &msg) {
}
void APIConnection::on_z_wave_proxy_request(const ZWaveProxyRequest &msg) {
enums::ZWaveProxyStatus status = zwave_proxy::global_zwave_proxy->zwave_proxy_request(this, msg.type);
// Only subscribe/unsubscribe are acknowledged (other types are server-to-client notifications)
if (msg.type == enums::ZWAVE_PROXY_REQUEST_TYPE_SUBSCRIBE ||
msg.type == enums::ZWAVE_PROXY_REQUEST_TYPE_UNSUBSCRIBE) {
ZWaveProxyRequestResponse resp{};
resp.type = msg.type;
resp.status = status;
if (!this->send_message(resp)) {
API_LOG_MSG_DROPPED(TAG, "Z-Wave proxy response");
}
ZWaveProxyRequestResponse resp{};
resp.type = msg.type;
resp.status = zwave_proxy::global_zwave_proxy->zwave_proxy_request(this, msg.type);
if (!this->send_message(resp)) {
API_LOG_MSG_DROPPED(TAG, "Z-Wave proxy response");
}
}
#endif
@@ -1567,10 +1562,14 @@ static enums::SerialProxyStatus serial_proxy_result_to_status(serial_proxy::Seri
switch (result) {
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_OK:
return enums::SERIAL_PROXY_STATUS_OK;
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_ASSUMED_SUCCESS:
return enums::SERIAL_PROXY_STATUS_ASSUMED_SUCCESS;
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE:
return enums::SERIAL_PROXY_STATUS_PORT_IN_USE;
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT:
return enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT;
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_TIMEOUT:
return enums::SERIAL_PROXY_STATUS_TIMEOUT;
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED:
return enums::SERIAL_PROXY_STATUS_NOT_SUPPORTED;
case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_ERROR:
@@ -1656,26 +1655,7 @@ void APIConnection::on_serial_proxy_request(const SerialProxyRequest &msg) {
status = serial_proxy_result_to_status(proxy->serial_proxy_request(this, msg.type));
break;
case enums::SERIAL_PROXY_REQUEST_TYPE_FLUSH:
// Flushing stalls the port, so it gets the same ownership check as writes
if (proxy->port_claimed_by_other(this)) {
status = enums::SERIAL_PROXY_STATUS_PORT_IN_USE;
break;
}
switch (proxy->flush_port()) {
case uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS:
status = enums::SERIAL_PROXY_STATUS_OK;
break;
case uart::UARTFlushResult::UART_FLUSH_RESULT_ASSUMED_SUCCESS:
status = enums::SERIAL_PROXY_STATUS_ASSUMED_SUCCESS;
break;
case uart::UARTFlushResult::UART_FLUSH_RESULT_TIMEOUT:
status = enums::SERIAL_PROXY_STATUS_TIMEOUT;
break;
case uart::UARTFlushResult::UART_FLUSH_RESULT_FAILED:
default:
status = enums::SERIAL_PROXY_STATUS_ERROR;
break;
}
status = serial_proxy_result_to_status(proxy->flush_port(this));
break;
default:
ESP_LOGW(TAG, "Unknown serial proxy request type: %" PRIu32, static_cast<uint32_t>(msg.type));
+1
View File
@@ -337,6 +337,7 @@ enum ZWaveProxyRequestType : uint32_t {
enum ZWaveProxyStatus : uint32_t {
ZWAVE_PROXY_STATUS_OK = 0,
ZWAVE_PROXY_STATUS_IN_USE = 1,
ZWAVE_PROXY_STATUS_NOT_SUPPORTED = 2,
};
#endif
#ifdef USE_SERIAL_PROXY
+2
View File
@@ -822,6 +822,8 @@ template<> const char *proto_enum_to_string<enums::ZWaveProxyStatus>(enums::ZWav
return ESPHOME_PSTR("ZWAVE_PROXY_STATUS_OK");
case enums::ZWAVE_PROXY_STATUS_IN_USE:
return ESPHOME_PSTR("ZWAVE_PROXY_STATUS_IN_USE");
case enums::ZWAVE_PROXY_STATUS_NOT_SUPPORTED:
return ESPHOME_PSTR("ZWAVE_PROXY_STATUS_NOT_SUPPORTED");
default:
return ESPHOME_PSTR("UNKNOWN");
}
@@ -92,7 +92,7 @@ void SerialProxy::dump_config() {
SerialProxyResult SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control,
uint8_t parity, uint8_t stop_bits, uint8_t data_size) {
#ifdef USE_API
if (this->port_claimed_by_other(api_connection)) {
if (this->port_claimed_by_other_(api_connection)) {
ESP_LOGW(TAG, "Ignoring configure request from client without port access [%" PRIu32 "]", this->instance_index_);
return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE;
}
@@ -154,7 +154,7 @@ void SerialProxy::write_from_client(api::APIConnection *api_connection, const ui
#ifdef USE_API
// Bytes from a client other than the live subscriber would interleave with the
// subscriber's traffic on the wire
if (this->port_claimed_by_other(api_connection)) {
if (this->port_claimed_by_other_(api_connection)) {
ESP_LOGW(TAG, "Ignoring write from client without port access [%" PRIu32 "]", this->instance_index_);
return;
}
@@ -166,13 +166,18 @@ void SerialProxy::write_from_client(api::APIConnection *api_connection, const ui
SerialProxyResult SerialProxy::set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) {
#ifdef USE_API
if (this->port_claimed_by_other(api_connection)) {
if (this->port_claimed_by_other_(api_connection)) {
ESP_LOGW(TAG, "Ignoring modem pin request from client without port access [%" PRIu32 "]", this->instance_index_);
return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE;
}
#endif
if (this->rts_pin_ == nullptr && this->dtr_pin_ == nullptr) {
ESP_LOGW(TAG, "No modem pins configured on serial proxy [%" PRIu32 "]", this->instance_index_);
// Asserting a pin that is not configured must fail so the client learns the signal never
// reached the wire; deasserting an absent pin is harmless and stays allowed
const uint32_t configured =
(this->rts_pin_ != nullptr ? static_cast<uint32_t>(SERIAL_PROXY_LINE_STATE_FLAG_RTS) : 0u) |
(this->dtr_pin_ != nullptr ? static_cast<uint32_t>(SERIAL_PROXY_LINE_STATE_FLAG_DTR) : 0u);
if ((line_states & ~configured) != 0) {
ESP_LOGW(TAG, "Requested modem pin not configured on serial proxy [%" PRIu32 "]", this->instance_index_);
return SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED;
}
const bool rts = (line_states & SERIAL_PROXY_LINE_STATE_FLAG_RTS) != 0;
@@ -195,13 +200,30 @@ uint32_t SerialProxy::get_modem_pins() const {
(this->dtr_state_ ? static_cast<uint32_t>(SERIAL_PROXY_LINE_STATE_FLAG_DTR) : 0u);
}
uart::UARTFlushResult SerialProxy::flush_port() {
SerialProxyResult SerialProxy::flush_port(api::APIConnection *api_connection) {
#ifdef USE_API
// Flushing stalls the port, so it gets the same ownership check as writes
if (this->port_claimed_by_other_(api_connection)) {
ESP_LOGW(TAG, "Ignoring flush from client without port access [%" PRIu32 "]", this->instance_index_);
return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE;
}
#endif
ESP_LOGV(TAG, "Flushing serial proxy [%" PRIu32 "]", this->instance_index_);
return this->flush();
switch (this->flush()) {
case uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS:
return SerialProxyResult::SERIAL_PROXY_RESULT_OK;
case uart::UARTFlushResult::UART_FLUSH_RESULT_ASSUMED_SUCCESS:
return SerialProxyResult::SERIAL_PROXY_RESULT_ASSUMED_SUCCESS;
case uart::UARTFlushResult::UART_FLUSH_RESULT_TIMEOUT:
return SerialProxyResult::SERIAL_PROXY_RESULT_TIMEOUT;
case uart::UARTFlushResult::UART_FLUSH_RESULT_FAILED:
return SerialProxyResult::SERIAL_PROXY_RESULT_ERROR;
}
return SerialProxyResult::SERIAL_PROXY_RESULT_ERROR; // Unreachable; all enum values handled above
}
#ifdef USE_API
bool SerialProxy::port_claimed_by_other(api::APIConnection *api_connection) const {
bool SerialProxy::port_claimed_by_other_(api::APIConnection *api_connection) const {
return this->api_connection_ != nullptr && this->api_connection_ != api_connection &&
this->api_connection_->is_connection_setup();
}
@@ -41,9 +41,11 @@ enum SerialProxyLineStateFlag : uint32_t {
/// Result of a client-initiated operation; mapped to api::enums::SerialProxyStatus by the API layer
enum class SerialProxyResult : uint8_t {
SERIAL_PROXY_RESULT_OK, ///< Operation completed or request accepted
SERIAL_PROXY_RESULT_ASSUMED_SUCCESS, ///< Platform cannot confirm TX drain; success assumed
SERIAL_PROXY_RESULT_PORT_IN_USE, ///< Denied: another live client holds the port
SERIAL_PROXY_RESULT_INVALID_ARGUMENT, ///< A parameter value is out of range
SERIAL_PROXY_RESULT_ERROR, ///< Driver or hardware error
SERIAL_PROXY_RESULT_TIMEOUT, ///< Timed out before TX completed
SERIAL_PROXY_RESULT_NOT_SUPPORTED, ///< Requested feature is not available on this instance
};
@@ -104,12 +106,8 @@ class SerialProxy final : public uart::UARTDevice, public Component {
uint32_t get_modem_pins() const;
/// Flush the serial port (block until all TX data is sent)
uart::UARTFlushResult flush_port();
#ifdef USE_API
/// True when a live subscriber other than the given connection holds the port
bool port_claimed_by_other(api::APIConnection *api_connection) const;
#endif
/// @param api_connection The API connection requesting the flush
SerialProxyResult flush_port(api::APIConnection *api_connection);
/// Set the RTS GPIO pin (from YAML configuration)
void set_rts_pin(GPIOPin *pin) { this->rts_pin_ = pin; }
@@ -121,6 +119,9 @@ class SerialProxy final : public uart::UARTDevice, public Component {
#ifdef USE_API
/// Read from UART and send to API client (slow path with 256-byte stack buffer)
void read_and_send_(size_t available);
/// True when a live subscriber other than the given connection holds the port
bool port_claimed_by_other_(api::APIConnection *api_connection) const;
#endif
/// Instance index for identifying this proxy in API messages
@@ -227,7 +227,7 @@ api::enums::ZWaveProxyStatus ZWaveProxy::zwave_proxy_request(api::APIConnection
default:
ESP_LOGW(TAG, "Unknown request type: %" PRIu32, static_cast<uint32_t>(type));
return api::enums::ZWAVE_PROXY_STATUS_OK;
return api::enums::ZWAVE_PROXY_STATUS_NOT_SUPPORTED;
}
}