Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wind] add a vertical component to the wind vector #1713

Merged
merged 4 commits into from Jun 14, 2016
Merged

[wind] add a vertical component to the wind vector #1713

merged 4 commits into from Jun 14, 2016

Conversation

gautierhattenberger
Copy link
Member

  • use new WIND_INFO message
  • update state interface

- use new WIND_INFO message
- update state interface
@flixr
Copy link
Member

flixr commented Jun 11, 2016

Wondering if it is nice to change the return type to struct instead of keeping it as pointer to struct like in all the other cases that don't return a scalar...

@gautierhattenberger
Copy link
Member Author

Returning a pointer is possible if we either only return the 3D wind vector or we split the horizontal and vertical part in state interface. In my mind, second option doesn't make much sense (although it avoids changing existing code, just extending it). First option needs to change a little bit the getter function (we can't really return a pointer to the horizontal part except with a brutal cast).

Actually, I don't mind to go back to pointers, but I did this for two reasons:

  • other elements of the air/wind group are already returned as scalars (airspeed, aoa, sideslip)
  • in a near future, I would like to make the state interface thread safe, which means returning pointers may not be possible anyway

@flixr
Copy link
Member

flixr commented Jun 13, 2016

Couldn't we just do something like

union windspeed_i {
  struct Int32Vect2 vect2;
  struct Int32Vect3 vect3;
}

and then still return pointers as &state.h_windspeed_i.vect2 or &state.h_windspeed_i.vect3 as needed?

I think that it is nicer to keep returning pointers for structs for now...
To make the state interface thread safe we also might want to consider other options?

@gautierhattenberger
Copy link
Member Author

back to the pointers.

Regarding the state interface, I'll open an issue first to discuss possible options.

@flixr
Copy link
Member

flixr commented Jun 13, 2016

Looks good to me and while I don't see why using the union would create problems it would be good to have it checked before?
Did you test it?

@gautierhattenberger
Copy link
Member Author

At least it works in simulation. I'll do more test in the coming weeks for a project, but it would help to have it in master already.

@flixr flixr merged commit 7e9fc87 into master Jun 14, 2016
@flixr flixr deleted the wind_info branch June 14, 2016 07:57
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.

None yet

2 participants