Hi Alex, Looks good. I’m going to go ahead and merge it and follow it up with a commit to make the GUI look a little better on MacOS. I’ll send another reply when they’re pushed to master.
Cheers, Jeff. > On 17 Jul 2019, at 23:45, Alexander Shuklin <jasura...@mail.ru> wrote: > > > Hi Jeff, > Have a nice flight. I found one more: > there's issue with menu item capitalization as well, so that will be in the > pcb_actions.cpp > > +//Show board statistics tool > +TOOL_ACTION PCB_ACTIONS::boardStatistics( > "pcbnew.InspectionTool.ShowStatisticsDialog", AS_GLOBAL, > + 0, LEGACY_HK_NAME( "Show Board Statistics" ), _( "Show Board > Statistics" ), > + _( "Shows board statistics" ), pcbnew_xpm ); > + > > https://github.com/jasuramme/kicad-source-mirror/commit/9d6061d5df63ee59f0c285cbe990cb7464e6f1da > > <https://github.com/jasuramme/kicad-source-mirror/commit/9d6061d5df63ee59f0c285cbe990cb7464e6f1da> > > Среда, 17 июля 2019, 23:56 +03:00 от Jeff Young <j...@rokeby.ie>: > > Hi Alex, > > I’m about to go on vacation myself so I’ll be on a plane all day tomorrow, > but I’ll look at this when I get there. > > Cheers, > Jeff. > > >> On 17 Jul 2019, at 19:06, Alexander Shuklin <jasura...@mail.ru >> <x-msg://e.mail.ru/compose/?mailto=mailto%3ajasura...@mail.ru>> wrote: >> >> Hi Jeff, >> >> That's not a problem. Now I'm on 2 weeks vacation )) >> We don't use header capitalization in russian language, so it's sometimes >> confuses me. >> All done. You can find changed commit on github if you want. >> >> https://github.com/jasuramme/kicad-source-mirror/commit/6ce77c20ff479f2fa4f1af9ba4e8d7f93d2ee1e5 >> >> <https://github.com/jasuramme/kicad-source-mirror/commit/6ce77c20ff479f2fa4f1af9ba4e8d7f93d2ee1e5> >> >> >> And patch in the attachment below. >> >> Best regards, >> Alex >> >> Среда, 17 июля 2019, 15:18 +03:00 от Jeff Young <j...@rokeby.ie >> <x-msg://e.mail.ru/compose/?mailto=mailto%3aj...@rokeby.ie>>: >> >> Hi Alex, >> >> No need for apologies. It’s a steep learning curve. >> >> Some more comments: >> >> 1) Dialog items use sentence-capitalization and colons; dialog headers use >> title-capitalization and no colons. So “Components:” should be >> “Components”, “Front side:” should be “Front Side”, etc. >> >> 2) Right-align the StdButtonSizer (and give it right and left margins). >> >> 3) I don’t think this ever made it into the coding guidelines, but we prefer >> ctor initializers each on their own line (with the colon on the line above): >> >> padsType_t( PAD_ATTR_T aAttribute, wxString aTitle ) : >> attribute( aAttribute ), >> title( aTitle ), >> qty( 0 ) >> >> 4) And yes, I would space-align the inspectMenu->AddItem calls that the Git >> checker messed up. >> >> If you don’t have time to make these fixes just holler and I can do it. >> >> Cheers, >> Jeff. >> >> >>> On 17 Jul 2019, at 12:47, Alexander Shuklin <jasura...@mail.ru <>> wrote: >>> >>> Hi, >>> I'm very sorry. I know, that's stupid, but it takes so much time to get >>> myself familiar with kicad code >>> If it's not too late, can you look for newer patch, not one I sent this >>> morning? Of course, I can make commit on top of it, but I think it doesn't >>> make sense >>> There only few changes, such as >>> - code formatting >>> - I used <>FinishDialogSettings() instead of just Fit() as written in >>> KiCad documentation >>> - I changed tool name for "pcbnew.InspectionTool.ShowStatisticsDialog" so >>> it would correspond to PCB_INSPECTION_TOOL::ShowStatisticsDialog method >>> - And I changed wxString ( _( "some string " ) ) to just _( "some string" ) >>> That's link to github: >>> https://github.com/jasuramme/kicad-source-mirror/commit/7846cc554fa13c27159d6b1ba4693ac96707c559 >>> >>> <https://github.com/jasuramme/kicad-source-mirror/commit/7846cc554fa13c27159d6b1ba4693ac96707c559> >>> >>> and patch uploaded to this mail >>> Sorry for that, but I've just understood that, during further doxygen >>> documentation reading >>> Среда, 17 июля 2019, 9:01 +03:00 от Alexander Shuklin <jasura...@mail.ru >>> <>>: >>> >>> Hi! >>> Sorry for delay, there to much to do for my primary job. >>> I hope that code will be alright. >>> That's commit on the github: >>> https://github.com/jasuramme/kicad-source-mirror/commit/9f9ff463ac679dd79a85a88c9b4156dcec05b337 >>> >>> <https://github.com/jasuramme/kicad-source-mirror/commit/9f9ff463ac679dd79a85a88c9b4156dcec05b337> >>> >>> There's one thing. Git coding standards checkers writes following >>> >>> +++ b/pcbnew/menubar_pcb_editor.cpp >>> @@ -436,7 +436,7 @@ void PCB_EDIT_FRAME::ReCreateMenuBar() >>> >>> inspectMenu->AddItem( PCB_ACTIONS::listNets, >>> SELECTION_CONDITIONS::ShowAlways ); >>> inspectMenu->AddItem( ACTIONS::measureTool, >>> SELECTION_CONDITIONS::ShowAlways ); >>> - inspectMenu->AddItem( PCB_ACTIONS::boardStatistics, >>> SELECTION_CONDITIONS::ShowAlways ); >>> + inspectMenu->AddItem( PCB_ACTIONS::boardStatistics, >>> SELECTION_CONDITIONS::ShowAlways ); >>> >>> >>> >>> But I would leave whitespaces, for better code looking as all functions >>> around use it >>> >>> >>> >>> Понедельник, 8 июля 2019, 17:03 +03:00 от Jeff Young <j...@rokeby.ie <>>: >>> >>> Hi Alex, >>> >>> Have a look at the second half of >>> DIALOG_GLOBAL_EDIT_TEXT_AND_GRAPHICS::TransferDataToWindow(). That code >>> builds a read-only grid. You could then have columns for Top, Bottom, and >>> Total. >>> >>> (BTW, I think we use Front/Back nomenclature rather than Top/Bottom.) >>> >>> Cheers, >>> Jeff. >>> >>> >>>> On 8 Jul 2019, at 13:48, Alexander Shuklin <jasura...@mail.ru >>>> <http://e.mail.ru/compose/?mailto=mailto%3ajasura...@mail.ru>> wrote: >>>> >>>> Hi Jeff, >>>> that bug reports are some job to do) >>>> >>>> Well, reason why I went up with that top-side/bottom-side checkboxes is if >>>> you will want to count let's say SMD components on the bottom side, you >>>> can manage it with setting top one off. I'm not sure, how important it is. >>>> On the another hand, we could write down something like: >>>> THT components: top: 15, bottom: 32, total: 47 >>>> SMD components: top: 14, bottom: 31, total: 45 >>>> But that would be ugly as well. >>>> >>>> If there would be more options and more components properties, maybe >>>> better to write all data to wxTextCtrl then? Just in big text field, which >>>> you could copy. And that will be easy to change and expand. >>>> >>>> Понедельник, 8 июля 2019, 14:24 +03:00 от Jeff Young <j...@rokeby.ie >>>> <http://e.mail.ru/compose/?mailto=mailto%3aj...@rokeby.ie>>: >>>> >>>> Hi Alex, >>>> >>>> I don’t like the top-side/bottom-side checkboxes (because I want to see >>>> both numbers at once), but I do like the other three. >>>> >>>> I think we need to move to a list or grid presentation, though. One of >>>> the other features we’ve talked about is user-defined attributes, so there >>>> might be more things than just THT, SMD and Virtual. >>>> >>>> Cheers, >>>> Jeff. >>>> >>>> [1] https://bugs.launchpad.net/kicad/+bug/1789363 >>>> <https://bugs.launchpad.net/kicad/+bug/1789363> >>>> [2] https://bugs.launchpad.net/kicad/+bug/980919 >>>> <https://bugs.launchpad.net/kicad/+bug/980919> >>>> >>>> >>>> >>>>> On 8 Jul 2019, at 11:20, Alexander Shuklin <jasura...@mail.ru <>> wrote: >>>>> >>>>> Hi!, I added stuff for footprints THT,SMD,virtual properties and now it's >>>>> calculate area. >>>>> But for me dialog is not very straight forward now. Can you please >>>>> advice, what better to do? >>>>> I attached 2 pics. I think, maybe right way is to add few checkboxes >>>>> which will change dialog data as soon as you pressed them? >>>>> Just look at pics and say what would be better, as I'm in doubt. >>>>> Second one is in WxFormBuilder so it looks a bit different. >>>>> >>>>> >>>>> Пятница, 5 июля 2019, 0:05 +03:00 от Jeff Young <j...@rokeby.ie <>>: >>>>> >>>>> 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 <>> >>>>>> 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 <> >>>>>> 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 >>>>>> <https://launchpad.net/~kicad-developers> >>>>>> Post to : 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> >>>>> >>>>> >>>>> >>>>> -- >>>>> Alexander Shuklin >>>>> <1.png><2.png> >>>> >>>> >>>> >>>> -- >>>> Alexander Shuklin >>> >>> >>> >>> -- >>> Alexander Shuklin >>> >>> >>> -- >>> Alexander Shuklin >>> <0001-added-board-statistics-dialog-which-shows-info-for-p.patch> >> >> >> >> -- >> Alexander Shuklin >> <0001-added-board-statistics-dialog-which-shows-info-for-p.patch> > > > > -- > Alexander Shuklin > <0001-added-board-statistics-dialog-which-shows-info-for-p.patch>
_______________________________________________ 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
