paparazzi-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Paparazzi-devel] Maximum speed?


From: Gautier Hattenberger
Subject: Re: [Paparazzi-devel] Maximum speed?
Date: Wed, 15 Apr 2015 23:37:53 +0200
User-agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

Hi,

Obviously the build process is a bit complicated, and probably worse for newcomers, undocumented. I'll try to make a draft of some schematics to explain how it is suppose to work and put it on the wiki. Any comments will be welcome then.

Gautier

Le 15/04/2015 04:00, Ori Pessach a écrit :
Suggestions? I'm not sure I can make any concrete suggestions that I feel very strongly about. The header vs. .c file issue is just a minor problem of initial friction (I looked for where something was being done, and it took me a while to realize that all the logic I was interested in was in a header file. It's an unusual choice, and not what I would expect.) Once you know where to find the code, it's not hard to understand, but there's some initial confusion where it seems like the code isn't actually there. 

A walkthrough is always nice to have, but like all documentation it would have to be maintained when the design changes. This rarely happens in practice, I find. It's better to write the code in a way that's readable and allows maintainers to discover where things are without having to grep through the entire code base for things that might be relevant.

The main source of confusion for me was how the entire airborne code is essentially compiled from the various XML files, but there's nothing in the XML files themselves that can give you an idea of how the resulting code is going to be structured. This is unlike any compiled system I've seen before, and feels more like a software framework, where user code fits into multiple interfaces and interacts with them in ways that may or may not be well documented - this kind of design tends to be difficult to understand, unless you have a thorough understanding of the entire system. There's just no obvious entry point.

Having said all that, I don't feel I have any good ideas on how to make things better. Not even if I were going to start from scratch (which no one here would want to do, and I'm obviously not advocating for that.) The goals of the project are certainly ambitious, and it attempts to solve a whole bunch of complicated problems, so it's not surprising that the solution is complicated. I guess I'm just frustrated at HOW complicated it is...




On Tue, Apr 14, 2015 at 10:14 AM, Felix Ruess <address@hidden> wrote:
Obviously I know the system quite well by now and hence it is kind of hard to take an unbiased and fresh look at it from the point of view from someone just starting...
IMHO it has improved quite a lot in the last years, making it more flexible and easier to use/understand. But then we also added a lot more things so it still ends up being quite complex due to the many options...

So what would you suggest? Getting input on how to improve this is certainly very welcome.
As I said, from my (biased) point of view, having generated functions in a header file instead of a .c file is not harder to understand... but maybe that's just me...

Do you think that "this sort of design decision" makes it harder to understand, or is it basically an issue of not having a good tutorial/walkthrough about how everything fits together?

On Tue, Apr 14, 2015 at 5:54 PM, Ori Pessach <address@hidden> wrote:
Well, this is exactly the problem with this sort of design decision - the only people who care about it are the ones struggling to understand what's going on because they're trying to make changes to the system. Once they figure it out, they lost their motivation to go in and fix things, presumably because they still want to make the change they had in mind before they went diving head first into the code. So the system always stays weird, the learning curve stays unpleasant, and potential maintainers without the tenacity to figure things out stay out of the developer pool. Some people see this as the desired outcome.

Personally, I don't feel very strongly about where the generated code lives. I was just pointing out that it was hard to find. I'm not likely to do anything about it - certainly when I don't have hardware that's capable of getting off the ground, which I currently don't...

On Tue, Apr 14, 2015 at 9:34 AM, Felix Ruess <address@hidden> wrote:
Well, I guess it is a bit a matter of preference...

To me having the auto_nav function as static inline in var/aircrafts/<ac_name>/ap/generated/flight_plan.h is no more difficult to understand or figure out than having it as a function in var/aircrafts/<ac_name>/ap/generated/flight_plan.c.
Even if the function would be in a generated C file, you would still have the generated waypoints, etc. in a header file...
And having it in a header file is slightly easier on the "build system" part, so you don't have to add sources to a generated directory in var...

Personally I really disliked that originally auto_nav was a macro, having it as a static inline function makes it much easier to debug (like a normal function).
I have no strong opinion about splitting it up and generate a .c and .h file, so if you want to implement it and make a pull request, I wouldn't mind merging it.
But it never bothered me enough to put in the effort to change it...

On Tue, Apr 14, 2015 at 5:13 PM, Ori Pessach <address@hidden> wrote:
It does make a difference to potential maintainers. It makes it difficult to figure out where the code ends up going, and the order of certain declarations, and it's certainly surprising.

On Tue, Apr 14, 2015 at 9:09 AM, Felix Ruess <address@hidden> wrote:
Not exactly sure why Pascal and Antoine originally decided to generate only a header that also includes the generated implementation.
But my guess is that it was just easier than generating both a .c and a .h file.
In the end it does not make a difference for the compiled binary...

On Tue, Apr 14, 2015 at 3:12 PM, Ori Pessach <address@hidden> wrote:
I'm still not clear on why all of the logic is in a generated header file... Was there a good reason for that?

On Tue, Apr 14, 2015 at 3:56 AM, Felix Ruess <address@hidden> wrote:
Hi Ori,

Well, a static inline function in a header file like that is essentially like a macro.
As I wrote before guidance_h_SetMaxSpeed is only there to adhere to the generated settings handler naming convention and used as an alias for gh_set_max_speed.
The compiler will inline and optimize this, so it is basically the same as if the generated settings functions would directly call gh_set_max_speed.
Could have been done using "#define guidance_h_SetMaxSpeed gh_set_max_speed" to the same effect...

To include a header file:
<header>
#include "guidance/guidance_h.h"
</header>

See also https://wiki.paparazziuav.org/wiki/Flight_Plans#Call

Cheers, Felix

On Mon, Apr 6, 2015 at 5:13 PM, Ori Pessach <address@hidden> wrote:
guidance_h_SetMaxSpeed() isn't a macro. It's an inline function, and it's declared static in a header file, which is not a great idea. In fact, putting all of the autopilot code in a header file is an odd choice...

How do I go about including a header from a flightplan, anyway?

--Ori

On Mon, Mar 23, 2015 at 3:30 PM, Felix Ruess <address@hidden> wrote:
The guidance_h_SetMaxSpeed() macro is basically only there to adhere to the naming convention for settings handlers.
You should be able to simply call gh_set_max_speed(speed) directly from your flightplan (but you probably need to include the header).

On Thu, Mar 12, 2015 at 3:35 PM, Ori Pessach <address@hidden> wrote:
This doesn't work as the code is currently written. guidance_h_SetMaxSpeed() is declared static (in a header file, no less) and the header isn't included in the generated header file that gets compiled into the autopilot firmware, so compilation fails because the reference to guidance_h_SetMaxSpeed() ends up being non-static.

Removing the static modifier gets the code to compile, but that's not ideal. 

The correct way to deal with this is to get the generated code to include the header file where guidance_h_SetMaxSpeed() is declared, but it doesn't seem like it's intended to be part of an official API (if one even exists...)

I have to wonder why the autopilot logic is generated into a header file. That seems like an odd (and potentially bad) choice. Is there a good reason for that?

Ori 

On Sun, Mar 8, 2015 at 2:38 PM, Sergey Krukowski <address@hidden> wrote:
Afaik, the idea was to use the function guidance_h_SetMaxSpeed:
https://github.com/paparazzi/paparazzi/blob/17b200a59e2fa52106fa973803f4952dadca8e97/sw/airborne/firmwares/rotorcraft/guidance/guidance_h.h#L117
Not sure however it there any other convenience functions for that.

Best Regards,
Sergey


The wiki has very little to say about setting the maximum speed:

http://wiki.paparazziuav.org/wiki/Flight_Plans#Dynacmically_adjustable_maximum_speed

Basically, nothing.

Is there a way to set the speed of travel between waypoints in a flightplan
for a rotorcraft?

Thanks,

Ori

_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel


_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel



_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel




_______________________________________________
Paparazzi-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/paparazzi-devel


reply via email to

[Prev in Thread] Current Thread [Next in Thread]