Temperature, catch bad latitude/longitude#3339
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves robustness of the TemperatureAPI coordinate resolution by handling non-numeric latitude/longitude values before attempting to use them for Open-Meteo requests.
Changes:
- Coerces
temperature_latitude/temperature_longitude(orzone.homefallbacks) to floats. - Logs and returns
(None, None)when coordinate values cannot be converted to floats.
| try: | ||
| latitude = float(latitude) | ||
| longitude = float(longitude) | ||
| except (TypeError, ValueError): | ||
| self.log("Warn: TemperatureAPI: Invalid latitude or longitude values: latitude {}, longitude {}".format(latitude, longitude)) |
There was a problem hiding this comment.
After coercing to float, values like NaN/Inf (e.g., float('nan')) or out-of-range coordinates (lat not in [-90, 90], lon not in [-180, 180]) will still pass and be used to build the API URL. Consider validating finiteness and bounds after float conversion and treating invalid values the same as the exception case (warn + return None, None) to avoid bad API requests.
| try: | ||
| latitude = float(latitude) | ||
| longitude = float(longitude) | ||
| except (TypeError, ValueError): |
There was a problem hiding this comment.
Because latitude/longitude are always either successfully converted to floats or the function returns in the except block, the later if latitude is not None and longitude is not None: check becomes redundant and the else branch is effectively unreachable. Consider handling the "missing" case before conversion (for clearer logging) or simplifying the control flow after the conversion.
| try: | ||
| latitude = float(latitude) | ||
| longitude = float(longitude) | ||
| except (TypeError, ValueError): | ||
| self.log("Warn: TemperatureAPI: Invalid latitude or longitude values: latitude {}, longitude {}".format(latitude, longitude)) |
There was a problem hiding this comment.
The new invalid-coordinate handling isn’t covered by tests: there are no cases for non-numeric values (e.g., 'unknown'), NaN/Inf, or out-of-range lat/lon. Please extend the existing Temperature API test suite to assert get_coordinates() / fetch_temperature_data() returns None and logs a warning for these inputs.
No description provided.