zayfod / pyfranca

A Python module and tools for working with Franca interface definition language (IDL) models.
MIT License
19 stars 13 forks source link

use namespace_references instead of package_references #25

Open jb090979 opened 7 years ago

jb090979 commented 7 years ago

This is a first draft of import rework. This request replaces the old one. My initial solution has mistakes by design -> forgot to consider circular package dependencies. Now this solution should handle the package import correctly.

The import behavior is changed. Now each namespace has a list of imported namepsace that are visible in the corresponding fidl file.

The unittest are green. But I think further test are neccessary. This draft should give you an impression of my solution. Any comment is welcome.

zayfod commented 7 years ago

Thanks, I am looking into the changes.

jb090979 commented 7 years ago

In order to show how the new package import is working I extend your fidl_dump.py modul. I add the --plot option to plot fidl dependencies in dot format.

I add plotting package and namespace dependencies. If you do not have a dot tool installed you can paste the dot code in http://www.webgraphviz.com/ and generate the image.

Example for a more complex import chain.

digraph " interface P.I " 
{
    "P.I"
    "P.I" -> "P.Type1";

    "P.Type1"
    "P.Type1" -> "P.PTypes";

    "P.PTypes"
    "P.PTypes" -> "P2.P2Types";

    "P2.P2Types"
    "P2.P2Types" -> "P3.P3Types";

    "P3.P3Types"
    "P.I" -> "P.Type2";

    "P.Type2"
    "P.Type2" -> "P.PTypes";
    "P.I" -> "P3.P3Types";
    "P.I" -> "P2.P2Types";
    "P.I" -> "P.PTypes";
}

and the corresponding package dependencies

digraph " P "
{
    "P"
    "P" -> "P2";
    "P2"
    "P2" -> "P3";
    "P3"
    "P" -> "P3";
}

Hopefully you like it.

zayfod commented 7 years ago

Hey, that's a really nice idea! :)

jb090979 commented 7 years ago

Hi

we have to decide to support or not support transistive imports. Currently transitive imports are not allowed. If you have a import chain like P1-NS1 --> P2-NS2 --> P3-NS3 Namespace1 from package 1 is not able to access Types of Namespsace3 of Package3.

What do you think?

kbirken commented 7 years ago

I suggest to stick with the existing semantics of Franca IDL as it is implemented in the Eclipse-based version. There, transitive imports will not work, i.e., from P1-NS1 only model elements from P2-NS2 will be visible. If P3-NS3 should be used from P1-NS1, the P3-NS3 file has to be imported.

BTW: The graphviz thingy is nice!

jb090979 commented 7 years ago

I'm glad to see that a franca developer join the conversation. Thanks for the advise. I like the decision not to support transitive imports.

The current implementation matches the behavior of the Eclipse-based version.

BTW: The graphviz thingy is nice!

I see that feature on other tools, like cmake and it really helps me to understand the structure of a bunch of fidl files.

Thank you

zayfod commented 7 years ago

Decided! :)

jb090979 commented 7 years ago

I add the next step. I redesign the fqn resolve method.

Now it detects ambiguous types and raise an exception in this case. Therfore no reference priority is necessary (or possible) anymore. Maybe later we can turn the exception into an warning and use the first finding.

If I have an interface like:

interface { typedef B is P2.TC.A typedef B2 is P.TC.A }

The corresponding type.name is "P2.TC.A" the resolver resolves this correctly. After resolving the fqn I replace the fqn just by the name "A" in this case. The benefit is taht tools which using pyfranca do not take care of a type was specified with or without . in typename. Unitest "test_type_notations" show the difference What do you think?

Any comment is welcome.

Job is finished.