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.mciner...@ieee.org> 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 Шуклин Александр <jasura...@mail.ru > <mailto:jasura...@mail.ru>> 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@lists.launchpad.net > <mailto:kicad-developers@lists.launchpad.net> > 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@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
