Skip to content

Move the SystemStructure from SymbolicAWEModels to KiteUtils#102

Closed
1-Bart-1 wants to merge 11 commits intomainfrom
system-structure
Closed

Move the SystemStructure from SymbolicAWEModels to KiteUtils#102
1-Bart-1 wants to merge 11 commits intomainfrom
system-structure

Conversation

@1-Bart-1
Copy link
Copy Markdown
Member

@1-Bart-1 1-Bart-1 commented Oct 25, 2025

This is necessary so KiteViewers can be updated to plot a SystemStructure.

  • Move SystemStructure
  • Add docs
  • Add tests

@1-Bart-1 1-Bart-1 requested a review from ufechner7 October 25, 2025 12:31
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 25, 2025

Codecov Report

❌ Patch coverage is 85.40146% with 20 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/system_structure.jl 85.29% 20 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Member

@ufechner7 ufechner7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, address the inline comments.

Comment thread src/system_structure.jl
const disturb::KVec3 # disturbing force
const force::KVec3
const type::DynamicsType
mass::SimFloat
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation of the fields mass and bridle_damping is missing. A physical unit must be provided. How can a point have a bridle_damping? Damping is a property of a segment, and not of a point.

Comment thread src/system_structure.jl
- `vel_w::KVec3=zeros(KVec3)`: Initial velocity of the point in world frame.
- `transform_idx::Int16=1`: Index of the transform used for initial positioning.
- `mass::Float64=0.0`: Mass of the point [kg].
- `bridle_damping::Float64=0.0`: Damping coefficient for bridle points.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A point cannot have a damping.

Comment thread src/system_structure.jl
mutable struct Group
const idx::Int16
const point_idxs::Vector{Int16}
const le_pos::KVec3 # point which the group rotates around under wing deformation
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this called le_pos? What does le stand for?

Comment thread src/system_structure.jl
# Arguments
- `idx::Int16`: Unique identifier for the group.
- `point_idxs::Vector{Int16}`: Indices of points that move together with this group's twist.
- `le_pos::KVec3`: Leading edge position in body frame.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The body frame is not defined at http://localhost:8000/reference_frames.html . Do not use reference frames without documenting how they are defined.

Comment thread src/system_structure.jl
- `point_idxs::Vector{Int16}`: Indices of points that move together with this group's twist.
- `le_pos::KVec3`: Leading edge position in body frame.
- `chord::KVec3`: Chord vector in body frame.
- `y_airf::KVec3`: Spanwise vector in local panel frame.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local panel frame is not defined at http://localhost:8000/reference_frames.html . Do not use reference frames without documenting how they are defined.

Comment thread src/system_structure.jl
aoa::SimFloat
fix_sphere::Bool
y_damping::SimFloat
z_disturb::SimFloat
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is z_disturb ?

Comment thread src/system_structure.jl
end

# Helper functions for quaternion/rotation matrix conversion
function quaternion_to_rotation_matrix(q::Vector{SimFloat})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function in this file and not in the file transformations.jl ?

Comment thread src/system_structure.jl
return R
end

function rotation_matrix_to_quaternion(R::Matrix{SimFloat})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function in this file and not in the file transformations.jl ?

Comment thread src/system_structure.jl
end

function Base.getproperty(wing::BaseWing, sym::Symbol)
if sym == :R_b_w
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is R_b_w? Where is it documented?

Comment thread src/system_structure.jl
# Arguments
- `idx::Int16`: Unique identifier for the wing.
- `group_idxs::Vector{Int16}`: Indices of groups attached to this wing.
- `R_b_c::Matrix{SimFloat}`: Rotation matrix from body frame to CAD frame.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the CAD frame? Do not use reference frames that are not documented in http://localhost:8000/reference_frames.html .

@1-Bart-1
Copy link
Copy Markdown
Member Author

Outdated

@1-Bart-1 1-Bart-1 closed this Apr 10, 2026
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.

3 participants