mirror of
				https://github.com/esphome/esphome.git
				synced 2025-11-04 00:51:49 +00:00 
			
		
		
		
	[api] Remove unnecessary string copies from optional access (#9897)
This commit is contained in:
		@@ -244,21 +244,7 @@ void APIConnection::loop() {
 | 
			
		||||
 | 
			
		||||
#ifdef USE_API_HOMEASSISTANT_STATES
 | 
			
		||||
  if (state_subs_at_ >= 0) {
 | 
			
		||||
    const auto &subs = this->parent_->get_state_subs();
 | 
			
		||||
    if (state_subs_at_ < static_cast<int>(subs.size())) {
 | 
			
		||||
      auto &it = subs[state_subs_at_];
 | 
			
		||||
      SubscribeHomeAssistantStateResponse resp;
 | 
			
		||||
      resp.set_entity_id(StringRef(it.entity_id));
 | 
			
		||||
      // attribute.value() returns temporary - must store it
 | 
			
		||||
      std::string attribute_value = it.attribute.value();
 | 
			
		||||
      resp.set_attribute(StringRef(attribute_value));
 | 
			
		||||
      resp.once = it.once;
 | 
			
		||||
      if (this->send_message(resp, SubscribeHomeAssistantStateResponse::MESSAGE_TYPE)) {
 | 
			
		||||
        state_subs_at_++;
 | 
			
		||||
      }
 | 
			
		||||
    } else {
 | 
			
		||||
      state_subs_at_ = -1;
 | 
			
		||||
    }
 | 
			
		||||
    this->process_state_subscriptions_();
 | 
			
		||||
  }
 | 
			
		||||
#endif
 | 
			
		||||
}
 | 
			
		||||
@@ -644,17 +630,13 @@ uint16_t APIConnection::try_send_climate_state(EntityBase *entity, APIConnection
 | 
			
		||||
  if (traits.get_supports_fan_modes() && climate->fan_mode.has_value())
 | 
			
		||||
    resp.fan_mode = static_cast<enums::ClimateFanMode>(climate->fan_mode.value());
 | 
			
		||||
  if (!traits.get_supported_custom_fan_modes().empty() && climate->custom_fan_mode.has_value()) {
 | 
			
		||||
    // custom_fan_mode.value() returns temporary - must store it
 | 
			
		||||
    std::string custom_fan_mode = climate->custom_fan_mode.value();
 | 
			
		||||
    resp.set_custom_fan_mode(StringRef(custom_fan_mode));
 | 
			
		||||
    resp.set_custom_fan_mode(StringRef(climate->custom_fan_mode.value()));
 | 
			
		||||
  }
 | 
			
		||||
  if (traits.get_supports_presets() && climate->preset.has_value()) {
 | 
			
		||||
    resp.preset = static_cast<enums::ClimatePreset>(climate->preset.value());
 | 
			
		||||
  }
 | 
			
		||||
  if (!traits.get_supported_custom_presets().empty() && climate->custom_preset.has_value()) {
 | 
			
		||||
    // custom_preset.value() returns temporary - must store it
 | 
			
		||||
    std::string custom_preset = climate->custom_preset.value();
 | 
			
		||||
    resp.set_custom_preset(StringRef(custom_preset));
 | 
			
		||||
    resp.set_custom_preset(StringRef(climate->custom_preset.value()));
 | 
			
		||||
  }
 | 
			
		||||
  if (traits.get_supports_swing_modes())
 | 
			
		||||
    resp.swing_mode = static_cast<enums::ClimateSwingMode>(climate->swing_mode);
 | 
			
		||||
@@ -1843,5 +1825,27 @@ uint16_t APIConnection::try_send_ping_request(EntityBase *entity, APIConnection
 | 
			
		||||
  return encode_message_to_buffer(req, PingRequest::MESSAGE_TYPE, conn, remaining_size, is_single);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
#ifdef USE_API_HOMEASSISTANT_STATES
 | 
			
		||||
void APIConnection::process_state_subscriptions_() {
 | 
			
		||||
  const auto &subs = this->parent_->get_state_subs();
 | 
			
		||||
  if (this->state_subs_at_ >= static_cast<int>(subs.size())) {
 | 
			
		||||
    this->state_subs_at_ = -1;
 | 
			
		||||
    return;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  const auto &it = subs[this->state_subs_at_];
 | 
			
		||||
  SubscribeHomeAssistantStateResponse resp;
 | 
			
		||||
  resp.set_entity_id(StringRef(it.entity_id));
 | 
			
		||||
 | 
			
		||||
  // Avoid string copy by directly using the optional's value if it exists
 | 
			
		||||
  resp.set_attribute(it.attribute.has_value() ? StringRef(it.attribute.value()) : StringRef(""));
 | 
			
		||||
 | 
			
		||||
  resp.once = it.once;
 | 
			
		||||
  if (this->send_message(resp, SubscribeHomeAssistantStateResponse::MESSAGE_TYPE)) {
 | 
			
		||||
    this->state_subs_at_++;
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
#endif  // USE_API_HOMEASSISTANT_STATES
 | 
			
		||||
 | 
			
		||||
}  // namespace esphome::api
 | 
			
		||||
#endif
 | 
			
		||||
 
 | 
			
		||||
@@ -298,6 +298,10 @@ class APIConnection : public APIServerConnection {
 | 
			
		||||
  // Helper function to handle authentication completion
 | 
			
		||||
  void complete_authentication_();
 | 
			
		||||
 | 
			
		||||
#ifdef USE_API_HOMEASSISTANT_STATES
 | 
			
		||||
  void process_state_subscriptions_();
 | 
			
		||||
#endif
 | 
			
		||||
 | 
			
		||||
  // Non-template helper to encode any ProtoMessage
 | 
			
		||||
  static uint16_t encode_message_to_buffer(ProtoMessage &msg, uint8_t message_type, APIConnection *conn,
 | 
			
		||||
                                           uint32_t remaining_size, bool is_single);
 | 
			
		||||
 
 | 
			
		||||
@@ -35,11 +35,10 @@ namespace esphome::api {
 | 
			
		||||
 *
 | 
			
		||||
 * Unsafe Patterns (WILL cause crashes/corruption):
 | 
			
		||||
 * 1. Temporaries: msg.set_field(StringRef(obj.get_string())) // get_string() returns by value
 | 
			
		||||
 * 2. Optional values: msg.set_field(StringRef(optional.value())) // value() returns a copy
 | 
			
		||||
 * 3. Concatenation: msg.set_field(StringRef(str1 + str2)) // Result is temporary
 | 
			
		||||
 * 2. Concatenation: msg.set_field(StringRef(str1 + str2)) // Result is temporary
 | 
			
		||||
 *
 | 
			
		||||
 * For unsafe patterns, store in a local variable first:
 | 
			
		||||
 *    std::string temp = optional.value();  // or get_string() or str1 + str2
 | 
			
		||||
 *    std::string temp = get_string();  // or str1 + str2
 | 
			
		||||
 *    msg.set_field(StringRef(temp));
 | 
			
		||||
 *
 | 
			
		||||
 * The send_*_response pattern ensures proper lifetime management by encoding
 | 
			
		||||
 
 | 
			
		||||
@@ -210,6 +210,15 @@ sensor:
 | 
			
		||||
    name: "Test Sensor 50"
 | 
			
		||||
    lambda: return 50.0;
 | 
			
		||||
    update_interval: 0.1s
 | 
			
		||||
  # Temperature sensor for the thermostat
 | 
			
		||||
  - platform: template
 | 
			
		||||
    name: "Temperature Sensor"
 | 
			
		||||
    id: temp_sensor
 | 
			
		||||
    lambda: return 22.5;
 | 
			
		||||
    unit_of_measurement: "°C"
 | 
			
		||||
    device_class: temperature
 | 
			
		||||
    state_class: measurement
 | 
			
		||||
    update_interval: 5s
 | 
			
		||||
 | 
			
		||||
# Mixed entity types for comprehensive batching test
 | 
			
		||||
binary_sensor:
 | 
			
		||||
@@ -285,6 +294,50 @@ valve:
 | 
			
		||||
    stop_action:
 | 
			
		||||
      - logger.log: "Valve stopping"
 | 
			
		||||
 | 
			
		||||
output:
 | 
			
		||||
  - platform: template
 | 
			
		||||
    id: heater_output
 | 
			
		||||
    type: binary
 | 
			
		||||
    write_action:
 | 
			
		||||
      - logger.log: "Heater output changed"
 | 
			
		||||
  - platform: template
 | 
			
		||||
    id: cooler_output
 | 
			
		||||
    type: binary
 | 
			
		||||
    write_action:
 | 
			
		||||
      - logger.log: "Cooler output changed"
 | 
			
		||||
 | 
			
		||||
climate:
 | 
			
		||||
  - platform: thermostat
 | 
			
		||||
    name: "Test Thermostat"
 | 
			
		||||
    sensor: temp_sensor
 | 
			
		||||
    default_preset: Home
 | 
			
		||||
    on_boot_restore_from: default_preset
 | 
			
		||||
    min_heating_off_time: 1s
 | 
			
		||||
    min_heating_run_time: 1s
 | 
			
		||||
    min_cooling_off_time: 1s
 | 
			
		||||
    min_cooling_run_time: 1s
 | 
			
		||||
    min_idle_time: 1s
 | 
			
		||||
    heat_action:
 | 
			
		||||
      - output.turn_on: heater_output
 | 
			
		||||
    cool_action:
 | 
			
		||||
      - output.turn_on: cooler_output
 | 
			
		||||
    idle_action:
 | 
			
		||||
      - output.turn_off: heater_output
 | 
			
		||||
      - output.turn_off: cooler_output
 | 
			
		||||
    preset:
 | 
			
		||||
      - name: Home
 | 
			
		||||
        default_target_temperature_low: 20
 | 
			
		||||
        default_target_temperature_high: 24
 | 
			
		||||
        mode: heat_cool
 | 
			
		||||
      - name: Away
 | 
			
		||||
        default_target_temperature_low: 16
 | 
			
		||||
        default_target_temperature_high: 26
 | 
			
		||||
        mode: heat_cool
 | 
			
		||||
      - name: Sleep
 | 
			
		||||
        default_target_temperature_low: 18
 | 
			
		||||
        default_target_temperature_high: 22
 | 
			
		||||
        mode: heat_cool
 | 
			
		||||
 | 
			
		||||
alarm_control_panel:
 | 
			
		||||
  - platform: template
 | 
			
		||||
    name: "Test Alarm"
 | 
			
		||||
 
 | 
			
		||||
@@ -4,7 +4,7 @@ from __future__ import annotations
 | 
			
		||||
 | 
			
		||||
import asyncio
 | 
			
		||||
 | 
			
		||||
from aioesphomeapi import EntityState, SensorState
 | 
			
		||||
from aioesphomeapi import ClimateInfo, EntityState, SensorState
 | 
			
		||||
import pytest
 | 
			
		||||
 | 
			
		||||
from .types import APIClientConnectedFactory, RunCompiledFunction
 | 
			
		||||
@@ -70,3 +70,22 @@ async def test_host_mode_many_entities(
 | 
			
		||||
        assert len(sensor_states) >= 50, (
 | 
			
		||||
            f"Expected at least 50 sensor states, got {len(sensor_states)}"
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
        # Get entity info to verify climate entity details
 | 
			
		||||
        entities = await client.list_entities_services()
 | 
			
		||||
        climate_infos = [e for e in entities[0] if isinstance(e, ClimateInfo)]
 | 
			
		||||
        assert len(climate_infos) >= 1, "Expected at least 1 climate entity"
 | 
			
		||||
 | 
			
		||||
        climate_info = climate_infos[0]
 | 
			
		||||
        # Verify the thermostat has presets
 | 
			
		||||
        assert len(climate_info.supported_presets) > 0, (
 | 
			
		||||
            "Expected climate to have presets"
 | 
			
		||||
        )
 | 
			
		||||
        # The thermostat platform uses standard presets (Home, Away, Sleep)
 | 
			
		||||
        # which should be transmitted properly without string copies
 | 
			
		||||
 | 
			
		||||
        # Verify specific presets exist
 | 
			
		||||
        preset_names = [p.name for p in climate_info.supported_presets]
 | 
			
		||||
        assert "HOME" in preset_names, f"Expected 'HOME' preset, got {preset_names}"
 | 
			
		||||
        assert "AWAY" in preset_names, f"Expected 'AWAY' preset, got {preset_names}"
 | 
			
		||||
        assert "SLEEP" in preset_names, f"Expected 'SLEEP' preset, got {preset_names}"
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user