Skip to content

Add altitude calculation from pressure time series#37

Merged
oyve merged 7 commits intodevfrom
copilot/add-altitude-calculation-feature
Dec 1, 2025
Merged

Add altitude calculation from pressure time series#37
oyve merged 7 commits intodevfrom
copilot/add-altitude-calculation-feature

Conversation

Copy link
Contributor

Copilot AI commented Nov 30, 2025

  • Understand the problem: Create a feature to calculate altitude from pressure time series data
  • Explore existing codebase and identify reusable functions
  • Run existing tests to verify current state
  • Implement altitudeFromPressureDifference() function in altitude.ts
  • Implement calculateAltitudesFromPressureSeries() function for time series data (removed per reviewer feedback)
  • Use sortOldestToNewest() in temperature.ts (reverted per reviewer feedback)
  • Add sortOldestToNewest() utility function in common.ts
  • Add JSDoc documentation with examples and references
  • Write comprehensive tests for new functions
  • Run linting, build, and tests (all 214 tests pass)
  • Manually verify the feature works correctly
  • Run code review
  • Run CodeQL security check
Original prompt

Suggest a new /altitude feature: To read pressure data and calculate the altitude based on the difference in pressure. This suggest having a time serie of data and pressure values, and a start altitude?


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: oyve <16225018+oyve@users.noreply.github.com>
Copilot AI changed the title [WIP] Add feature to calculate altitude from pressure data Add altitude calculation from pressure time series Nov 30, 2025
Copilot AI requested a review from oyve November 30, 2025 13:08
@oyve
Copy link
Owner

oyve commented Nov 30, 2025

@copilot Use the Reading class in common.cs

…g interface

Co-authored-by: oyve <16225018+oyve@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 30, 2025

@copilot Use the Reading class in common.cs

Updated to use the Reading interface from common.ts instead of a custom PressureReading interface. The function now takes Reading[] as input and returns Reading[] with calculated altitudes. Commit: 8f53a83

@oyve
Copy link
Owner

oyve commented Nov 30, 2025

@copilot See if any formulas can be reused, i.e. altitudeFromPressureDifference - could it already exist in Temperature.ts or Pressure.ts?

Copy link
Contributor Author

Copilot AI commented Nov 30, 2025

@copilot See if any formulas can be reused, i.e. altitudeFromPressureDifference - could it already exist in Temperature.ts or Pressure.ts?

Reviewed temperature.ts and pressure.ts for reusable formulas:

  • pressureAltitude() in pressure.ts calculates ISA-based altitude from a single pressure value, but doesn't support pressure differences or temperature compensation
  • barometricFormula() in pressure.ts uses similar constants (R, g, scaleHeight) but calculates pressure from altitude (the inverse operation)

The altitudeFromPressureDifference() function uses the hypsometric equation which is the inverse of the barometric formula, specifically designed for calculating altitude change from two pressure readings with temperature compensation. While the underlying physics constants are shared (already using DRY_AIR_CONSTANTS from constants.ts), the mathematical operations are different enough that the new function provides distinct functionality.

Would you prefer I move this function to pressure.ts to keep altitude/pressure calculations together, or keep it in altitude.ts?

@oyve
Copy link
Owner

oyve commented Nov 30, 2025

@copilot move sort logic to common.cs

Co-authored-by: oyve <16225018+oyve@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 30, 2025

@copilot move sort logic to common.cs

Moved sort logic to common.ts as a reusable sortReadingsByTimestamp() utility function. Also updated temperature.ts to use this new function for consistency. Commit: 028ab7a

@oyve
Copy link
Owner

oyve commented Nov 30, 2025

@copilot rename sortReadingsByTimestamp to sortOldestToNewest

Co-authored-by: oyve <16225018+oyve@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 30, 2025

@copilot rename sortReadingsByTimestamp to sortOldestToNewest

Renamed sortReadingsByTimestamp to sortOldestToNewest in common.ts and updated all usages in altitude.ts and temperature.ts. Commit: c15c987

@oyve
Copy link
Owner

oyve commented Nov 30, 2025

@copilot Reomve the pressureSeries logic

Co-authored-by: oyve <16225018+oyve@users.noreply.github.com>
@oyve
Copy link
Owner

oyve commented Nov 30, 2025

@copilot undo changes to filterReadingsByTimeRange

Co-authored-by: oyve <16225018+oyve@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 30, 2025

@copilot undo changes to filterReadingsByTimeRange

Reverted filterReadingsByTimeRange to its original implementation with inline sorting. Commit: a071cbd

@oyve
Copy link
Owner

oyve commented Dec 1, 2025

@copilot Could pressureAltitude in pressure.ts be used? Could the barometric formula be used? (it's in pressure.ts).

Copy link
Contributor Author

Copilot AI commented Dec 1, 2025

@copilot Could pressureAltitude in pressure.ts be used? Could the barometric formula be used? (it's in pressure.ts).

Analyzed both options:

pressureAltitude: Could be used as pressureAltitude(P2) - pressureAltitude(P1), but it assumes ISA standard temperature (15°C). The result doesn't change with actual temperature conditions.

barometricFormula: Calculates pressure from altitude (the inverse operation). Would need to be mathematically inverted to solve for altitude, which is effectively what the hypsometric equation does.

The key advantage of the current altitudeFromPressureDifference implementation is temperature compensation:

  • At 0°C: 515m altitude for same pressure change
  • At 30°C: 572m altitude for same pressure change
  • pressureAltitude diff: Always 540m regardless of temperature

Should I simplify by using pressureAltitude differences (losing temperature compensation), or keep the current temperature-aware implementation?

@oyve oyve marked this pull request as ready for review December 1, 2025 12:41
@oyve oyve merged commit 15eddcf into dev Dec 1, 2025
7 checks passed
@oyve oyve deleted the copilot/add-altitude-calculation-feature branch December 1, 2025 12:44
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.

2 participants