Do not fail when a part is missing

[leoheck]

Do not fail when a part is missing

[jnavila]

It's feasible and it's indeed a useful degraded mode.

[leoheck]

If you have any idea on how to implement this, you could try to guide me in making these changes if you don't have much time or energy.

[jnavila]

It may be a bit difficult for a first issue. I may have some time tomorrow for this. If you still want to try and tackle it, here is the line where the component is looked for in the library and where the exception is raised:

plotkicadsch/kicadsch/src/kicadlib.ml

Lines 345 to 347 in a8afb21

	let thecomp =
	try Lib.find lib (fix_illegal_chars comp_name)
	with _ -> raise (Component_Not_Found comp_name)

So we have to add a parameter to the function, and bypass the raising of exception in case of component not found. Something in the lines of:

  let plot_comp (lib, _, _) comp_name part rotation origin fail_on_missing (ctx : drawContext)
      =
    let rot = rotate rotation origin in
    try let thecomp =
      Lib.find lib (fix_illegal_chars comp_name)
      in
      ( List.fold_left (plot_elt rot thecomp part) ctx thecomp.graph
    , thecomp.multi )
    with _ -> if fail_on_missing then raise (Component_Not_Found comp_name) else (ctx, false)

Then, the parameter must be passed on at the level of kicadsch.ml

plotkicadsch/kicadsch/src/kicadsch.ml

Lines 395 to 396 in a8afb21

	let canevas', is_multi =
	CPainter.plot_comp lib sym m_unitnr origin transfo

and then from

plotkicadsch/kicadsch/src/kicadsch.ml

Line 718 in a8afb21

let parse_line line (ctx:schContext) =

and up from:

plotkicadsch/plotkicadsch/src/kicadDiff.ml

Lines 37 to 38 in a8afb21

	let process_file initctx filename =
	let parse c l = S.parse_line l c in

where parse_line is used, and up again to all prototypes and call sites of process_file. Then the option can be added to:

plotkicadsch/plotkicadsch/src/kicadDiff.ml

Line 104 in a8afb21

let doit from_fs to_fs file_to_diff differ textdiff libs keep colors zone_color =

and finally as a command line parameter to plotgitsch.ml. Of course, the interface change of the kicadlib will also affect plotkicadsch.

So the core of the change is trivial, but the management of the option is quite work intensive. Good news is, OCaml won't let the compilation finish until everything typechecks correctly, so once it compiles, it's almost sure to work as expected.

If you're still interested, I recommend that you use VS Code and the OCaml mode to help you with the typing.

[jnavila]

Ah, the interface files (*.mli) need also to be updated.

[leoheck]

Alright, I was struggling to put everything you said working. And it looks you put clear instructions for me here. So the big barrier now is the language and the syntax. Every little thing I do doesn't compile.

Trying a new approach. If I got everything right, I could ignore this because most of the part is just to make the tool user-friendly. So to test the functionality, I can then:

plotkicadsch/kicadsch/src/kicadlib.ml

Line 347 in a8afb21

with _ -> raise (Component_Not_Found comp_name)

With this

with _ -> (ctx, false)

Then I removing this

plotkicadsch/kicadsch/src/kicadlib.ml

Line 340 in a8afb21

exception Component_Not_Found of string

I was able to compile it without failing.

And WOW. It works. Finally, I could make a change in OCaml. This was hard.
Now I am not seeing exceptions but the plot itself.

Interesting that I am not seeing any point where the missing component is.
But I am not using Plotgitsch to compare. I am using plotgitsch to generate images only.

And this is the issue I was seeing for this design.

plotgitsch-dir -d ./ -l .//board-cache.lib -c FF0000:008FFF:FFFFFF:222222 5d48ef7 7e1baef # &> /dev/null
Use compare (ImageMagick) between bitmaps between Git rev 5d48ef7 and Git rev 7e1baef
Exception Kicadsch__Kicadlib.MakePainter(P).Component_Not_Found("Connector_Generic:Conn_01x03")

[jnavila]

Try removing the -l .//board-cache.lib option.

[leoheck]

It seems to be working. So does it mean the library is not needed anymore?

➜ plotgitsch -c FF0000:008FFF:FFFFFF:222222 5d48ef7 e0bb0ff

Use compare (ImageMagick) between bitmaps between Git rev 5d48ef7 and Git rev e0bb0ff
Exporting...
sch1-board.svg (e0005d)
sch2-board.svg (1348a8)

Ah, this is the output of my git-imgdiff

This is the project that was giving the exception. The project is not at the root of the repo too.

What did you expected to see?

[jnavila]

Adding the library would help find components in case the cache lib has not been committed.

About my expectations, assuming that the connector's component was missing in the cache lib of oldest revision, I would expect it to not be drawn there, while still present in the newest one, so the diff would show the component's shape being added, but not its reference and label which are drawn at the sheet level.

What did you expect?

[leoheck]

Adding the library would help find components in case the cache lib has not been committed.

Oh, so the -l is not for the -cache-lib since you are saying cache lib is already the default place for looking for missing parts.

What should I use with the -l then? For instance, my project uses too many libraries. Does this flag accept a list of files or a duplication of the same flag? I am using it as default, to make sure I will have a plot of the schematics but with the -cache-lib instead.

Alternatively, maybe you are saying that it tries to use the cached.lib from the commit's history by default, but if you pass -l it will use this current file (latest version of this file) instead of or as the alternative.

I am not pretty sure if I got the whole behavior of this -l flag

What did you expect?

The same as you described. But I am not sure if that has happened.
For instance, I am comparing (actually generating images for) the current commit with every other commit in the whole git history.
Then I am comparing the images using an image comparing tool externally (the one in Kicad-Diff, for instance)
So, I am not sure if I saw a diff where I have this missing part in blue/red indicating that it was missing.

I am going to double-check it.

[leoheck]

My commit tree just has 7 commits.

This is the HEAD vs HEAD^1

This is the HEAD vs 1st commit
This image shows both versions have this missing header J1.

This is the HEAD vs 3rd commit

I was expecting to see at least one diff that shows a missing header, but none of them was. And also, at least one with the header in blue but no red one (or the opposite, with the header in red but the blue missing).

[jnavila]

The -l option preloads the given lib (be it cache or not) from the file system before trying to find the -cache.lib in the given commit for each commit to compare. You can add several --lib=<library.lib> options on your command line and all the libs will be preloaded in turn for both versions to compare.

From your screen capture
https://user-images.githubusercontent.com/1277920/118813550-a4cd2100-b885-11eb-8c0e-bcf9f40e9622.png

We can see the missing connector. So, I'm not quite sure what we are looking for here. Maybe could you share a minimal project that exhibits your issue.

Anyway, I can implement the correct option management for the change you've made if you find it useful.

[leoheck]

Sure, I will try to create the project with this issue. I cannot share the one I was sharing. Yes, please. Implement it if you have time. I am using my fork of plotgitsch now but I would prefer to use your so people will have the original instead. [Off topic] I will also create another issue to have a flag to keep intermediate files even if the diff does not have any changes. This is also important for me. And may be good for someone else, and it may no to be complex to implement (if you are imolenting, of course). I could not make it then I just removed a test that was making this restriction.

…

On Wed, May 19, 2021, 13:56 Jean-Noël Avila ***@***.***> wrote: The -l option preloads the given lib (be it cache or not) from the file system before trying to find the -cache.lib in the given commit for each commit to compare. You can add several --lib=<library.lib> options on your command line and all the libs will be preloaded in turn for both versions to compare. From your screen capture https://user-images.githubusercontent.com/1277920/118813550-a4cd2100-b885-11eb-8c0e-bcf9f40e9622.png We can see the missing connector. So, I'm not quite sure what we are looking for here. Maybe could you share a minimal project that exhibits your issue. Anyway, I can implement the correct option management for the change you've made if you find it useful. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#51 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJX7YHJJDEMIIESRFR3NMLTOPUVXANCNFSM45A5GOKA> .

[leoheck]

You were right. The image you have referenced shows the missing part. It happened that the change in that specific commit was in the same spot and I got confused a bit.

[jnavila]

fixed by c7513d1