← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Board statistics dialog

 

I agree that it shouldn’t be its own tool, but PCB_EDITOR_CONTROL is getting too big.

Let’s just name this tool the PCB_INSPECTION_TOOL, and then I’ll move the highlight and list nets stuff, and the DRC dialog to it later.  That’ll nicely parallel the EE_INSPECTION_TOOL.

Cheers,
Jeff.

> On 4 Jul 2019, at 21:56, Ian McInerney <Ian.S.McInerney@xxxxxxxx> wrote:
> 
> This looks nice, but a few comments from my initial usage of it:
> 
> Usability:
> 1) When I use english units, your statistics dialog gives the dimensions in mils. It should probably give those in inches instead, since that is what the grid panel at the bottom gives.
> 2) Including the board area would also be useful, definitely the area of the bounding box, and possibly the area of the actual PCB (but that is more complicated). I know of several board houses that charge based on area, so this could provide a quick way to estimate price for the user.
> 3) I am not sure I am a fan of the icon used for the menu item, since this dialog focuses on board statistics rather than footprint statistics and that icon is used mainly for footprint actions. Could a variant of the pcbnew icon be used?
> 
> 
> While the code as you gave works, I think the following changes could be beneficial:
> 1) It seems that this action could just be added to PCB_EDITOR_CONTROL as the action "pcbnew.Control.BoardStatistics" rather than creating a new tool just for this.
> 2) I think the launching of the window could be simplified if you used a modal window instead (see https://docs.wxwidgets.org/3.0/classwx_dialog.html <https://docs.wxwidgets.org/3.0/classwx_dialog.html>, the Modal and Modeless section). Then you don't have to keep track of the window pointer, and could actually just make launching this a single function instead of an entire class (this would go nicely with point #1).
> 3) The function to start the window should be TransferDataToWindow, not TransferDataFromWindow. FromWindow is called when the window is destroyed and ToWindow is called when it is created. Also, it is called automatically when the window is started, so there is no need to manually call it.
> 
> 
> Other code parts:
> 1) There are some lines that are too long in dialog_board_statistics.cpp (max 100 characters per line)
> 2) There are a few whitespace errors when applying it:
> Applying: added board statistics dialog, which shows info for production and assembly
> /home/imcinerney/Downloads/0001-added-board-statistics-dialog-which-shows-info-for-p.patch:158: trailing whitespace.
>                 /* if pin has drill with width==0 and height==0, we 
> /home/imcinerney/Downloads/0001-added-board-statistics-dialog-which-shows-info-for-p.patch:1612: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
> 
> Overall though it looks pretty nice.
> 
> -Ian
> 
> On Thu, Jul 4, 2019 at 3:07 PM Шуклин Александр <jasuramme@xxxxxxx <mailto:jasuramme@xxxxxxx>> wrote:
> Hi, that's first time I try to contribute to KiCad and write to Launchpad mailing lists, so please, don't beat me to hard )))
> I really miss some board statistic dialog, where you can see quantity of SMD pads, THT pads, board dimensions, all the stuff, you need for PCB production and assembly. There was also issue in the bug tracker 
> https://bugs.launchpad.net/kicad/+bug/1817232 <https://bugs.launchpad.net/kicad/+bug/1817232>
> And like guy from bug issue, I moved from Altium Designer and miss that dialog as well. 
> Can you please look at that and commit if you think it's useful or tell me what to change.
> That's my commit in the github:
> https://github.com/jasuramme/kicad-source-mirror/commit/6290375c1d41ddb89d4b08067593f170c7d344c5 <https://github.com/jasuramme/kicad-source-mirror/commit/6290375c1d41ddb89d4b08067593f170c7d344c5>
> and branch:
> https://github.com/jasuramme/kicad-source-mirror/tree/statistic_dialog <https://github.com/jasuramme/kicad-source-mirror/tree/statistic_dialog>
> and there's also patch and dialogs pics in the attachment.
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
> More help   : https://help.launchpad.net/ListHelp <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