FredsFactory / FreeCAD_AirPlaneDesign

FreeCAD WorkBench Air Plane Design
GNU Lesser General Public License v2.1
79 stars 14 forks source link

Code doubts/suggestions #19

Closed adrianinsaval closed 4 years ago

adrianinsaval commented 4 years ago

Hi! After reading some of the codes I have a few notes:

https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/8e4222c2846ece61cea2547e1e32d0541569004d/airPlaneAirFoil.py#L138 Shouldn't this segment be something like:

coords,geometry = readpointsonfile(filename)
if len(coords) == 0 :
    print("list of points are empty")

Or I'm missunderstanding it's purpose? Shouldn't it somehow throw an error message?

https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/8e4222c2846ece61cea2547e1e32d0541569004d/airPlaneAirFoil.py#L121 Instead of a for, you could just use:

coords = upper
coords.extend(lower)

https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/8e4222c2846ece61cea2547e1e32d0541569004d/airPlaneAirFoil.py#L123 Is this correct? Isn't geometry having the lower side written twice?

https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/8e4222c2846ece61cea2547e1e32d0541569004d/airPlaneAirFoil.py#L126 What is this seemingly random vector?

Also what is the need to store the points in two different variables (coordsand geometry)?

FredsFactory commented 4 years ago

thanks for reading the code and you found some errors.

What is this seemingly random vector? I deleted the random vector that I use for some tests! 8-)

What is also the need to store the points in two different variables (coordinates and geometry)?

This is a test in order to integrate the set of points into the geometry property. It works, I plan to have only one variable in the next version.

I leave the question open for now Thank you again for your help

adrianinsaval commented 4 years ago

Nice, thanks for the answer. Should development be made using coords or geometry?

FredsFactory commented 4 years ago

geometry is the standard property of a object, but the function return a chords and not a coords, not easy to decide ... 8-)

adrianinsaval commented 4 years ago

Hi @FredsFactory ! I noticed in airPlaneAirFoil.py geometry is now = to coords, should this be applied in airPlaneAirFoilNaca.py too? is it necessary at all to define geometry there? couldn't coords simply be returned by the function instead of geometry?

EDIT: Seems geometry has different type of data depending on if the airfoil points were flipped or not: https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/3774da4a5902d8de12f22771b0461eca080d3aa5/airPlaneAirFoil.py#L86 https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/3774da4a5902d8de12f22771b0461eca080d3aa5/airPlaneAirFoil.py#L111

adrianinsaval commented 4 years ago

The ambiguous definition of geometry also causes a bug where a rib that was defined by .dat file cannot be recomputed, the issue arises when geometry is passes to process() as coords and it is not read again from the file: https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/3774da4a5902d8de12f22771b0461eca080d3aa5/airPlaneRib.py#L92 https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/3774da4a5902d8de12f22771b0461eca080d3aa5/airPlaneRib.py#L97 https://github.com/FredsFactory/FreeCAD_AirPlaneDesign/blob/3774da4a5902d8de12f22771b0461eca080d3aa5/airPlaneAirFoil.py#L114 Will submit a Pull Request soon fixing this and some small improvements

EDIT: PR submited