← Back to team overview

kicad-developers team mailing list archive

Re: Pcbnew display origin transforms for v6

 

Hi Wayne,

No worries on formatting. As a senior professional software engineer I'm used to dealing with formal coding conventions and I'm keeping the web page open while I'm writing. I prefer the K&R style myself, but I'm not going to lose sleep over the differences.

I had a bit of trouble getting the clang-format commit checker to work. Mint 18 installs clang-format 3.8 by default, which doesn't support the 'BreakStringLiterals' key. It spits out the following message:

YAML:23:22: error: unknown key 'BreakStringLiterals'
BreakStringLiterals: false
                     ^~~~~
Error reading /home/reece/MyProjects/KiCad/kicad/_clang-format: Invalid argument

With the invocation of clang-format buried under the git extension mechanism this error message is invisible to the user. Worse, clang-format then blithely ignores the entire style file and reformats the file using its default settings (IndentWidth 2, etc.). The notes should probably be updated to note the minimum version of clang-format required to handle the supplied _clang-format file.

-Reece


On 5/2/19 7:56 AM, Wayne Stambaugh wrote:
Hi Reece,

Just a few comments on top of Jeff's reply since this will be your first
patch submission:

Please follow the coding style policy[1].  It saves a lot of back and
forth.  There is also a git commit hook[2] which you can use to verify
your coding style is correct.  You will have to have clang-format
installed on your system in order to use this feature.

When you are ready to submit your patch(es), use `git format-patch` to
create the patch(es) and post them on the mailing list for comment.

It was great talking with you at KiCon.  Thank you again for your
interest in contributing to KiCad.

Cheers,

Wayne

[1]:
http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html
[2]:
http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html#tools

On 4/30/19 9:50 PM, Reece R. Pollack wrote:
Inspired by KiCon (and before the high wears off) I'm moving forward
with my project to allow the user to specify the coordinate display
origin in pcbnew. I have this working as patches to the v5.1.2 branch,
but they're hard-coded to use the Aux origin. I'd like some guidance on
how best to code this for eventual inclusion into the v6 development branch.

My plan is:

  1. Create a new class ORIGIN_TRANSFORM. This class maintains the user's
     configuration and provides functions to perform the transforms
     necessary to convert between the internal coordinate representation
     and the equivalent coordinates relative to the user-selected origin.
  2. Embed two ORIGIN_TRANSFORM objects in the BOARD_DESIGN_SETTINGS
     class representing the origin transforms for the X and Y axes.
  3. Create a new tab in the pcbnew "Preferences" to allow the user to
     select the origin (Page [default], Aux, or Grid) and direction (Left
     or Right [default], Up or Down [default]) for each axis.
  4. Modify the board file format to save and load the user's
     ORIGIN_TRANSFORM configuration. Saving this in the .kicad_pcb file
     allows everyone editing the board to see coordinates reported
     relative to the same origin.
  5. Modify each of the dialog classes that displays object coordinates
     to use the ORIGIN_TRANSFORM objects to display coordinates relative
     to the user-selected origin. When the dialog is closed object
     coordinates are converted back to internal coordinates.
  6. Modify the PCB_BASE_FRAME class to display the absolute cursor
     position relative to the user-selected origin.

This is structured to allow the UNIT_BINDER class to manage origin
transforms, much like it handles the mm/mil conversions. I would change
the constructor to take a reference to an ORIGIN_TRANSFORM object as an
additional argument; the default value would be a unity-transform whose
conversion functions return the value to be converted unmodified. Those
UNIT_BINDER objects that represent a coordinate value (commonly m_posX
and m_posY) would be constructed with a reference to one of the two
ORIGIN_TRANSFORM objects in the BOARD_DESIGN_SETTINGS class.

The problem with this strategy is that KiCad reuses UNIT_BINDER objects
for dissimilar purposes. For example,
DIALOG_GRAPHIC_ITEM_PROPERTIES::m_endX can represent the end point of a
graphic line, for which origin transform is needed. But it can also
represent the radius of a circle, for which origin transform is
inappropriate. Thus UNIT_BINDER needs a method similar to the Show()
function to enable or disable the origin transform depending on how it
is being used. This may argue against pushing this functionality into
UNIT_BINDER.

If I don't modify the UNIT_BINDER class I'll probably code the
ORIGIN_TRANSFORM class to handle wxPoint objects rather than individual
coordinates. Thus there would be only one ORIGIN_TRANSFORM object added
to the BOARD_DESIGN_SETTINGS class. The transform in the dialog class
then becomes a single call to transform a coordinate pair rather than
one each for X and Y.

Thoughts? Comments? Suggestions? Brickbats?

-Reece



_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp



Follow ups

References