Menu

#488 check command and libanalyze function to do the work of rtcheck

Unstable
closed-accepted
5
2018-05-25
2018-05-22
Sharan
No

This is a check command that uses a newly implemented libanalyze function - analyze_overlaps.
The check command does what rtcheck does but uses a function instead of the current implementation that uses an executable version of rtcheck.
The libanalyze function takes a callback function and callback data, that is used to fill up the local overlap list of ged_check with overlaps using a local function (add_overlaps).
This function is called by a adapter which has the same prototype as a_overlap expects.

2 Attachments

Discussion

  • Sharan

    Sharan - 2018-05-22

    Describing the two patches:
    1) check_bu_exit.patch - this has everything as it is with bu_exit for any errors that occur, I found that a bit harsh for MGED to exit for something like a typo when giving the command so I made a new patch with using return and making all the functions int type instead of void, then evaluating these return values.
    2) check_return.patch - As described above, also I noticed that rpt_overlaps feature was not working so I removed the -r option because the existing rtcheck sets a_logoverlap to rt_silent_logoverlap in view_init making the -r option redundant. (The first patch doesn't have this removed)

     
  • Daniel Roßberg

    Daniel Roßberg - 2018-05-22

    OK, stay with the check_return.patch.
    - Most important, I would like to see the object selection in ged_check().
    - Next important, using application.a_overlap is deprecated. See include/rt/application.h.
    - I would give the add_overlaps parameter of analyze_overlaps() in include/analyze.h a different, more general, name. For example simply "callback" or "overlapHandler". It should signal the user that they can implement anything here, not just adding the overlap to a list.
    Otherwise, it looks good, I would say.

     

    Last edit: Daniel Roßberg 2018-05-22
  • Sharan

    Sharan - 2018-05-22

    Following changes:
    1) Moved object selection to libged.
    2) Moved do_ae and grid_setup to libged, resulting in a smaller parameter list for libanalyze :)
    3) Changed overlapsAdapter to the match a_multioverlap defintion.
    4) Renamed add_overlaps in analyze.h to callback.

     

    Last edit: Sharan 2018-05-22
  • Sharan

    Sharan - 2018-05-25

    Following changes:
    1) Using a_logoverlap instead of a_multioverlap for passing the overlapsAdapter.
    2) Doing processing in libged only.
    3) Renamed command to check_overlaps.
    4) Renamed files, libged/check.c to libged/check_overlaps.c and libanalyze/overlaps/check_overlaps.c to libanalyze/overlaps/analyze_overlaps.c

     
  • Daniel Roßberg

    Daniel Roßberg - 2018-05-25

    To speed it up I accepted your patch, but it has some minor issues:
    In include/analyze.h:
    - newline at line 128
    - name the parameters in analyze_overlaps_callback, such that the user hasn't to guess what they mean

    In src/libanalyze/overlaps/analyze_overlaps.c:
    - remove spaces in line 326

    In src/libtclcad/tclcad_obj.c:
    - use a tab

    Test with Archer.

    Please, correct them with an own patch.

     
  • Daniel Roßberg

    Daniel Roßberg - 2018-05-25
    • status: open --> closed-accepted
    • Group: Incomplete --> Unstable
     

Log in to post a comment.