Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On Wed, Feb 9, 2011 at 5:35 PM, Wayne Stambaugh stambau...@verizon.net wrote: It looks good. It certainly is a huge improvement over the old dialog. Aside from some minor alignment issues the only comment I have is what does the Apply Settings button do? If it just copies the dialog settings to application variables, why not do this on close if there are any changes? If it does more than that, then a more descriptive button name may be useful. The apply settings button is a remnant from the old dialog. I does what you thought it would do. I guess the original idea was to make it possible to close the dialog without saving some goofy settings. Currently the settings are saved when plot or apply settings is clicked. But you're right. We probably can survive without the apply settings button. marco ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
For me the dialog is very very good, and works as expected. Still the issue remains that the dialog can't be closed with the window button and the cancel button. I hacked a class function DIALOG_PLOT_BASE::OnQuit with EndModal(0); and it worked. Maybe the virtual functions need be implemented? I saw also the eeschema postscript dialog is derived from the wxFormBuilder base class and there are the virtual functions implemented and have not problem with this. Sorry, but I'm not to got yet with C++ so don't blame my bad thinking. Jerry On 2/2/11 8:26 AM, jean-pierre charras wrote: Le 02/02/2011 00:15, Marco Mattila a écrit : Now there should be a little something for everyone in the latest revision. It was possible to make the plot dialog a little smaller by getting rid of the radio buttons and moving the output directory field (although I'm not sure it's in the right place now...). I also agree that the dialog is a bit too square. It's hard to make the height smaller. And it doesn't make sense to make the window wider than necessary. I can later take a look at how using wxCheckListBox affects the layout. Wayne, you did mean that widthheight, right? Jerry asked about saving the outputdirectory in the board file. That's the way it has been for a few revisions already. marco Plot dialog looks good! Is the dialog is a bit too square ? Remember it size depend on platform: You cannot have *exactly* the right size, because the actual size of a dialog depend on system settings and the system itself (Language, font size, how widgets are drawn by the window manager...) ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On Thu, Feb 3, 2011 at 6:32 PM, Jerry Jacobs xor.gate.engineer...@gmail.com wrote: For me the dialog is very very good, and works as expected. Still the issue remains that the dialog can't be closed with the window button and the cancel button. I hacked a class function DIALOG_PLOT_BASE::OnQuit with EndModal(0); and it worked. Maybe the virtual functions need be implemented? I committed a small patch. It seems that the name of a callback was wrong. Is it any better now? marco ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Hello Marco, I like it but it has still some issues with wxWidgets on Mac with version 2.9.2 from svn (rev 66766). I put a screenshot on my server over here: http://kicad.xor-gate.org/screenshots/new-plot-screen-issues.png The issues are the format frame, and the gerber options entries labels overlap. Also I can't close the window with the exit button on the window border, and the same goes for the close button. So I have to kill pcbnew to quit the application. A suggestion would also be to display the cwd in the Output directory line if the user opens the dialog, and maybe this output directory can be saved/loaded from the pcbfile/project. On 2/1/11 7:10 AM, jean-pierre charras wrote: Le 31/01/2011 21:55, Marco Mattila a écrit : Jean-Pierre, I have yet another suggestion for the layout of the plot dialog (see attachment). The width would be the same as in your recent patch, but the height would be smaller. What do you think? marco I like it! ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Marco, Now please realize that you have to make EVERYONE happy, even if they disagree among themselves. :) And no good deed goes unpunished. And the pay is low. And coder decides. Because the pay is low. Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 2/1/2011 6:15 PM, Marco Mattila wrote: Now there should be a little something for everyone in the latest revision. It was possible to make the plot dialog a little smaller by getting rid of the radio buttons and moving the output directory field (although I'm not sure it's in the right place now...). I also agree that the dialog is a bit too square. It's hard to make the height smaller. And it doesn't make sense to make the window wider than necessary. I can later take a look at how using wxCheckListBox affects the layout. Wayne, you did mean that widthheight, right? Not necessarily. It's just the ratio of 1.6 to 1 seems to be pleasing to the human eye. Artists and architects have known this for a long time. Check out Wikipedia's page. It's one of those strange things that just seems to work. Wayne Jerry asked about saving the outputdirectory in the board file. That's the way it has been for a few revisions already. marco ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Le 31/01/2011 21:55, Marco Mattila a écrit : Jean-Pierre, I have yet another suggestion for the layout of the plot dialog (see attachment). The width would be the same as in your recent patch, but the height would be smaller. What do you think? marco I like it! -- Jean-Pierre CHARRAS ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Le 29/01/2011 22:01, Marco Mattila a écrit : On Sat, Jan 29, 2011 at 10:24 PM, Marco Mattilamarco...@gmail.com wrote: I've been rearranging the plot dialog. The current status is shown in the attachment. There should be enough room to add a checkbox for marking the path relative. On second thought, it should be possible to infer from the path whether it is relative or not (starts with / or not (in linux)). In this case relative to the project directory, I guess. Need to check how wxWidgets deals with paths. marco Most of time plot subdirectory is a subdirectory of the current working directory. Mainly in this case (easy to detect), the stored path must be relative to the CWD. Do not forget the CWD can easily change for a given project( for instance when you copy/move/save your projects) Do do forget to store paths names in unix like form, i.e. replace \ by / (if any). This ensure Kicad files compatibility between platforms, especially for subdirectories in the CWD, because (unlike the full path) the relative path usually does not depend of platform. Thanks. -- Jean-Pierre CHARRAS ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/30/2011 07:20 AM, jean-pierre charras wrote: Le 29/01/2011 22:01, Marco Mattila a écrit : On Sat, Jan 29, 2011 at 10:24 PM, Marco Mattilamarco...@gmail.com wrote: I've been rearranging the plot dialog. The current status is shown in the attachment. There should be enough room to add a checkbox for marking the path relative. On second thought, it should be possible to infer from the path whether it is relative or not (starts with / or not (in linux)). In this case relative to the project directory, I guess. Need to check how wxWidgets deals with paths. marco Most of time plot subdirectory is a subdirectory of the current working directory. Mainly in this case (easy to detect), the stored path must be relative to the CWD. Do not forget the CWD can easily change for a given project( for instance when you copy/move/save your projects) Do do forget to store paths names in unix like form, i.e. replace \ by / (if any). This ensure Kicad files compatibility between platforms, especially for subdirectories in the CWD, because (unlike the full path) the relative path usually does not depend of platform. Thanks. Jean-Pierre, What is the the established convention on the CWD while running the various Kicad programs? Marco will need to understand this, and any code is dependent on (hopefully) solid assumptions about where CWD is pointing. Thanks, Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On Sun, Jan 30, 2011 at 3:20 PM, jean-pierre charras jp.char...@wanadoo.fr wrote: Most of time plot subdirectory is a subdirectory of the current working directory. Mainly in this case (easy to detect), the stored path must be relative to the CWD. Do not forget the CWD can easily change for a given project( for instance when you copy/move/save your projects) Do do forget to store paths names in unix like form, i.e. replace \ by / (if any). This ensure Kicad files compatibility between platforms, especially for subdirectories in the CWD, because (unlike the full path) the relative path usually does not depend of platform. I made a commit a while ago. Now the output directory can be absolute or relative to the board file path. I trust wxFileName to do the heavy lifting when it comes to compatibility of relative paths between operating systems. The only thing done in this regard in the pcbplot.cpp code is the replacement of backslashes with forward slashes. Windows should be ok with that. All this needs testing in windows, of course. marco ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
I will fix number 1. It is my problem anyway. Dick The null string output directory is fixed. You might still get the exception reading the board file, but only until you save the board file again. The desired content of the board file with the null string output directory is now with double quotes indicating the null string: (outputdirectory ) PcbPlotParams IDs this line. To achieve this sanity, I had to fully decouple our quoting protocol from the Specctra DSN one. We can now control our own destiny, with less worry of breaking the interface to Specctra DSN. See Documentation/s-expression.txt Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Marco, I went back and took a look at your commit one last time, and I again want to complement on your excellence. It is *extremely* good, nice looking work. It is clear that your experience with C++ is substantial, and I appreciate the degree to which you have been willing to conform to established practices. Thanks Marco! Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Le 29/01/2011 15:33, Dick Hollenbeck a écrit : Marco, I went back and took a look at your commit one last time, and I again want to complement on your excellence. It is *extremely* good, nice looking work. It is clear that your experience with C++ is substantial, and I appreciate the degree to which you have been willing to conform to established practices. Thanks Marco! Dick Thanks also, Marco. But I must say there are 2 issues which must be solved: 1 - if outputdirectory is empty ( ie: (outputdirectory ) ),a PARSE_ERROR is displayed. Should not. Easy to reproduce: open the plot dialog, erase the output directory field, save params, save board file and reopen it. 2 - Each time relative paths can be used, use and store relative path. This is a *golden rule*. I am talking from experience. Here is my reason: 1 - this is the easier way to keep compatibility between Windows/Linux/MacOS in .brd files. The same working directory can have different name. 2 - in many cases users do not use only one computer, for instance in schools. The absolute path of the working directory is not always (or never) known. For instance, files can be stored on an USB key, and the working directory does not have the same path on each computer (this is a common case). 3 - Use absolute paths can create tricky bugs: for instance: you have a project called myproject. You want to test a minor change, so to do it, you create a new project called myproject1, that is the copy of the first project. Because an absolute path is stored in the .brd file, all plot files created by the new project will be written in the first project (and break the first project p^lot files). For an user, this is the kind of issue that is very difficult to see. -- Jean-Pierre CHARRAS ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On Sat, Jan 29, 2011 at 10:24 PM, Marco Mattila marco...@gmail.com wrote: I've been rearranging the plot dialog. The current status is shown in the attachment. There should be enough room to add a checkbox for marking the path relative. On second thought, it should be possible to infer from the path whether it is relative or not (starts with / or not (in linux)). In this case relative to the project directory, I guess. Need to check how wxWidgets deals with paths. marco ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
I might even use a macro, not a function, and have it be a single character macro name, like N(), TN(), GTN(). I also offer: n() which seems to get out of the way even less. ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On Fri, Jan 28, 2011 at 12:21 AM, Dick Hollenbeck d...@softplc.com wrote: Any complex string that is capable of having either whitespace or quotes in it should be wrapped using aFormatter-Quoted( CONV_TO_UTF8(blah) ).c_str(), I'm working on this now. Should there be something like if( strchr( quote_char, *wrapee ) ) return quote_char; in the for loop of OUTPUTFORMATTER::GetQuotedChar? Otherwise it seems that a string with internal quotes is not wrapped in quotes. marco ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/28/2011 11:18 AM, Marco Mattila wrote: On Fri, Jan 28, 2011 at 12:21 AM, Dick Hollenbeck d...@softplc.com wrote: Any complex string that is capable of having either whitespace or quotes in it should be wrapped using aFormatter-Quoted( CONV_TO_UTF8(blah) ).c_str(), I'm working on this now. Should there be something like if( strchr( quote_char, *wrapee ) ) return quote_char; in the for loop of OUTPUTFORMATTER::GetQuotedChar? Otherwise it seems that a string with internal quotes is not wrapped in quotes. Here are two things, either of which would define a problem: 1) Is it incompatible with the what freerouter expects or can tolerate? 2) Is it incompatible with what the DSNLEXER expects or can tolerate? I don't know that the condition you state meets either of these criteria, but I will investigate and get back to you. The Quoted() function was written to make DSNLEXER happy and however unusual things look there, as long as those two code bodies agree, then it may not be an issue. What does it mean to agree? This is the round-tripping of 8 bit text to and from the quoted form, those two strings need to agree. This may not look like what you expect or want on disk. Having said that, then are you saying a string like this: ABCD On the way out, *before* quoting will be a problem? Will not come back in via DSNLEXER? Have you verified this problem string with actual testing? Thanks, Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/28/2011 01:18 PM, Marco Mattila wrote: On Fri, Jan 28, 2011 at 8:11 PM, Dick Hollenbeck d...@softplc.com wrote: Having said that, then are you saying a string like this: ABCD On the way out, *before* quoting will be a problem? Will not come back in via DSNLEXER? Have you verified this problem string with actual testing? I'm saying that the comments in OUTPUTFORMATTER::Quoted say that any string which has internal quotes will also be wrapped in quotes later in this function. If my string (in this case outputDirectory) is ABCD it is going on disk as ABCD. Based on the comment I think it should be ABCD. Anyways, if it is on disk as ABCD it comes back as ABCD. If it is on disk as ABCD it comes back as ABCD. If that is how things are supposed to work, I can strip the extra 's in PCB_PLOT_PARAMS_PARSER. The round trip criteria is not being met, according to you. So there is a problem. You should get back in from DSNLEXER what you send out through OUTPUTFORMATTER::Quoted().c_str(). Plain and simple. I will test, verify and fix. I also am switching over to Quoted() in template_fieldnames.cpp, which probably gave you some wrong direction in the first place. Won't have time to go the 2nd step and introduce TEMPLATE_FIELDNAMES_PARSER which I plan to do later, so that we can continue to be consistent with the same design pattern. Nice job. Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/28/2011 11:18 AM, Marco Mattila wrote: On Fri, Jan 28, 2011 at 12:21 AM, Dick Hollenbeck d...@softplc.com wrote: Any complex string that is capable of having either whitespace or quotes in it should be wrapped using aFormatter-Quoted( CONV_TO_UTF8(blah) ).c_str(), I'm working on this now. Should there be something like if( strchr( quote_char, *wrapee ) ) return quote_char; Marco, I just now committed a fix. I looked at your suggestion, but took a slightly more conservative approach, since I wanted no part of breaking the specctra export code. We use a slightly different quoting protocol over there, since that area cannot be as well defined as our stuff, nor is it as well documented. Then you have Alfon's interpretation blended into whatever you might find in the DSN spec. So that is not solid footing, and unnecessary changes are evil. There is no notion of double quoting in the DSN format, that is all stuff I added on top of the s-expression stuff, our so called quoting protocol, which in essence, is DEFINED by OUTPUTFORMATTER::Quoted(). So that explains why the function Quoted() is not being used over there, in any specctra export code. The quoting protocol is our own, and WE should use Quoted() moving forward. And if you wonder where our documentation is, you have been reading it recently, in the source code and in our emails. Source code is actually a very accurate form of documentation, albeit not universally understood. Thanks for your patience and fine work. Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Very nice work. What you have for saving and loading seems future proof now, i.e. context free. Just some comments below, but I think you are already at 99% perfection. === modified file 'CMakeLists.txt' -add_subdirectory(gerbview) add_subdirectory(kicad) add_subdirectory(pcbnew) +add_subdirectory(gerbview) Above: Don't like alphabetical? { Line = aReader-Line(); + +if( strnicmp( Line, PcbPlotParams, 13 ) == 0 ) +{ +PCB_PLOT_PARAMS_PARSER parser( Line[13], aReader-GetSource() ); + +try +{ +g_PcbPlotOptions.Parse( parser ); +} +catch( IO_ERROR e ) +{ +D( printf( pcbplotparams parsing error: '%s'\n, + CONV_TO_UTF8( e.errorText ) ); ); We can probably do better than this, using wxMessageWhatever() +} + +continue; +} + === added file 'pcbnew/pcb_plot_params.cpp' --- pcbnew/pcb_plot_params.cpp1970-01-01 00:00:00 + +++ pcbnew/pcb_plot_params.cpp2011-01-27 14:23:38 + @@ -0,0 +1,383 @@ + +/* + * This program source code file is part of KICAD, a free EDA CAD application. + * + * Copyright (C) 1992-2011 Kicad Developers, see change_log.txt for contributors. If this is your code, put your name here if you want. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + Following function is bound to conflict in global namespace, make static and shorten name considerably is suggested. If its static and local, you could make it EXTREMELY short even a just a couple of characters. + +const char* GetTokenName( T aTok ) +{ +return PCB_PLOT_PARAMS_LEXER::TokenName( aTok ); +} + + + +void PCB_PLOT_PARAMS::Format( OUTPUTFORMATTER* aFormatter, + int aNestLevel ) const throw( IO_ERROR ) +{ +const char* falseStr = GetTokenName( T_false ); +const char* trueStr = GetTokenName( T_true ); Any complex string that is capable of having either whitespace or quotes in it should be wrapped using aFormatter-Quoted( CONV_TO_UTF8(blah) ).c_str(), not the way you are doing it. The lexer will not like what you are doing in all cases, that is the reason for Quoted(). This pertains to outputDirectory below, but it is a general statement. +aFormatter-Print( aNestLevel+1, (%s \%s\)\n, GetTokenName( T_outputdirectory ), + CONV_TO_UTF8( outputDirectory ) ); +aFormatter-Print( 0, )\n ); +} + Is this operator=() different than what the compiler will automatically generate for you? + + +PCB_PLOT_PARAMS PCB_PLOT_PARAMS::operator=( const PCB_PLOT_PARAMS aPcbPlotParams ) +{ +layerSelection = aPcbPlotParams.layerSelection; +useGerberExtensions = aPcbPlotParams.useGerberExtensions; +m_ExcludeEdgeLayer = aPcbPlotParams.m_ExcludeEdgeLayer; +m_PlotLineWidth = aPcbPlotParams.m_PlotLineWidth; +m_PlotFrameRef = aPcbPlotParams.m_PlotFrameRef; +m_PlotViaOnMaskLayer = aPcbPlotParams.m_PlotViaOnMaskLayer; +m_PlotMode = aPcbPlotParams.m_PlotMode; +useAuxOrigin = aPcbPlotParams.useAuxOrigin; +m_HPGLPenNum = aPcbPlotParams.m_HPGLPenNum; +m_HPGLPenSpeed = aPcbPlotParams.m_HPGLPenSpeed; +m_HPGLPenDiam = aPcbPlotParams.m_HPGLPenDiam; +m_HPGLPenOvr = aPcbPlotParams.m_HPGLPenOvr; +m_PlotPSColorOpt = aPcbPlotParams.m_PlotPSColorOpt; +m_PlotPSNegative = aPcbPlotParams.m_PlotPSNegative; +m_PlotReference = aPcbPlotParams.m_PlotReference; +m_PlotValue = aPcbPlotParams.m_PlotValue; +m_PlotTextOther = aPcbPlotParams.m_PlotTextOther; +m_PlotInvisibleTexts = aPcbPlotParams.m_PlotInvisibleTexts; +m_PlotPadsOnSilkLayer = aPcbPlotParams.m_PlotPadsOnSilkLayer; +subtractMaskFromSilk = aPcbPlotParams.subtractMaskFromSilk; +m_PlotFormat = aPcbPlotParams.m_PlotFormat; +m_PlotMirror = aPcbPlotParams.m_PlotMirror; +m_DrillShapeOpt = aPcbPlotParams.m_DrillShapeOpt; +scaleSelection = aPcbPlotParams.scaleSelection; +outputDirectory =
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 1/4/2011 10:31 PM, Dick Hollenbeck wrote: The previous patch has a bug. It incorrectly complains about no layers being selected. This patch works better. marco On Sun, Jan 2, 2011 at 11:40 PM, Marco Mattila marco...@gmail.com wrote: Hi, During previous discussions about subtracting masks from silkscreen layers when plotting gerbers, Dick mentioned that saving plot settings is not quite up to date. Currently some plot settings to be saved/loaded are defined in pcbnew_config.cpp. However, that requires that corresponding variables in the PCB_PLOT_PARAMS class are public. My proposal is to let the class itself take care of saving/loading the settings. That's mainly what the attached patch changes. In addition, all plot dialog settings should now be included. The wxConfig key names are also edited to start with Plot, and layer selections are combined into a single bit mask. Moreover, some global variables in pcbplot.cpp have been moved into the class, too. g_PcbPlotOptions itself is still global. However, I think that it could be moved to be a member of the WinEDA_BasePcbFrame. That way it should be accessible where needed without being global. And if necessary, plot settings could still be loaded only once when pcbnew starts (now they are loaded every time the plot dialog constructor is called). In addition drill file generation settings could be included into PCB_PLOT_PARAMS. I can continue working on this if my approach sounds reasonable. marco I would say the patch looks like an improvement worth committing. Thanks. If anyone has concerns they should voice them. This is a nice patch so I'm all for committing it. I would have liked it even more if there was a way to avoid using a the global variable g_PcbPlotOptions. Thanks Marco for your help. Wayne This little bit here is goofy though, for a function comment: PCB_PLOT_PARAMS(); +/** + Function LoadSettings + loads plot settings from wxConfig system. + @param aConfig is a pointer to a wxConfig. + */ +voidLoadSettings( wxConfig* aConfig ); +/** + Function SaveSettings + saves current plot settings to wxConfig system. + @param aConfig is a pointer to a wxConfig. + */ +void In the future, please leave a space between these like the coding standard says, and fill in the vertical stars. My vote is to commit. Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
Damn. I always struggle the most with the comments for Doxygen... I think that we may be able to get rid of the global variable. I'll take a look. I'll fix the comments in the next patch, too. marco On Wed, Jan 5, 2011 at 3:24 PM, Wayne Stambaugh stambau...@verizon.net wrote: This is a nice patch so I'm all for committing it. I would have liked it even more if there was a way to avoid using a the global variable g_PcbPlotOptions. Thanks Marco for your help. ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/05/2011 07:47 AM, Marco Mattila wrote: Damn. I always struggle the most with the comments for Doxygen... I think that we may be able to get rid of the global variable. I'll take a look. I'll fix the comments in the next patch, too. marco From a usability standpoint, i.e. user experience, I have a concern about saving the desired layers, as if I was only ever going to work on one board. Now that you have a compact bitmap, would it make sense to prefix the Config key for layers, with the boardname? Although this might eventually pollute the configuration space, you'd have to do a lot boards to get there. This way it remembers what layers you want for what boards. The idea would be to put this information in the board, no preference here. Is this anything worth considering? Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
From a usability standpoint, i.e. user experience, I have a concern about saving the desired layers, as if I was only ever going to work on one board. Now that you have a compact bitmap, would it make sense to prefix the Config key for layers, with the boardname? Although this might eventually pollute the configuration space, you'd have to do a lot boards to get there. This way it remembers what layers you want for what boards. The alternative idea would be to put this information in the board, no preference here. Is this anything worth considering? Dick Dick, English please. ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/05/2011 09:10 AM, Dick Hollenbeck wrote: From a usability standpoint, i.e. user experience, I have a concern about saving the desired layers, as if I was only ever going to work on one board. Now that you have a compact bitmap, would it make sense to prefix the Config key for layers, with the boardname? I have a couple of 2 layer boards and a 4 layer board I work on, intermittently. I can tolerate all the same settings for both boards at gerber generation time, but the number and types of layers are the main difference. The proposed solution handles my situtation, which is why I suggested it. :) And it has hi return on investment, because it's easy to do. Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
If you think that the plotting requirements vary from one project to another (the number of needed technical layers maybe?), saving them outside the config system makes sense. Since there can be multiple boards in a project, maybe the SETUP section of a board file is the correct place for this. I wonder if all plot settings should be saved in the board file. For example, it would be nice if the output directory was saved, too. marco On Wed, Jan 5, 2011 at 6:29 PM, jp.charras jp.char...@wanadoo.fr wrote: Le 05/01/2011 16:10, Dick Hollenbeck a écrit : From a usability standpoint, i.e. user experience, I have a concern about saving the desired layers, as if I was only ever going to work on one board. Now that you have a compact bitmap, would it make sense to prefix the Config key for layers, with the boardname? Although this might eventually pollute the configuration space, you'd have to do a lot boards to get there. This way it remembers what layers you want for what boards. The alternative idea would be to put this information in the board, no preference here. Is this anything worth considering? Dick An other idea is to use the .pro file to store layers. So, no config polluted. Currently Pcbnew already updates the corresponding .pro file when it is closed. ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
On 01/05/2011 11:23 AM, Marco Mattila wrote: If you think that the plotting requirements vary from one project to another (the number of needed technical layers maybe?), saving them outside the config system makes sense. Since there can be multiple boards in a project, maybe the SETUP section of a board file is the correct place for this. I wonder if all plot settings should be saved in the board file. For example, it would be nice if the output directory was saved, too. marco If you were to write a Parse() and Format() function for your PLOT settings object, similar to those in new/sch_lib_table.cpp or class TEMPLATE_FIELD_NAMES, then you could stuff or retrieve the data almost anywhere, and it would be re-usable moving forward. This includes the ability to: 1) save it in the current board file, all on one line. 2) read it from the current board file format, all from one line. 3) save it to any new S-EXPRESSION based board file in the future. 4) read it from any new S-EXPRESSION based board file in the future. 5) Save/read it to the clipboard. For 1) and 2) you would need an old school prefix on the BOARD file line, such as: // for current board file format, all on one LONG line: PlotConfigs: (plot_conf () () ()) For an example, see what I did for eeschema, TEMPLATE_FIELD_NAMES: *) eeschema_config.cpp, line 711, where I save the s-expression. *) eeschema_config.cpp, line 628, where I read the s-expression. If any new data structure were to have these Parse() and Format() functions, it becomes really pretty easy to serialize these things almost anywhere, including to the clipboard, or nested in side another S-EXPRESSION. This would give you an intro to the make_lexer support in the CMake files. If you were going to parse everything off of a single line, then probably the STRING_LINE_READER constructor of the DSNLEXER derivative is what could be used for stuffing this stuff all on one line in a current board file. This is what I did for TEMPLATE_FIELD_NAMES. The storage location is almost a separate discussion from the storage format. I like the s-expression direction we have, obviously, and I don't see any big problems putting it into a BOARD file, but I could be wrong. The only concern is maximum line length, but if the BOARD reader were to use a FILE_LINE_READER, then that concern goes away, since FILE_LINE_READER will read single lines of up to 100,000 bytes. Or you can simply not inject FILE_LINE_READER into the BOARD loader, but verify that its fgets() buffer is big enough to handle your new long single line. Jean-Pierre, what do you think of this as a migration path, or even as a middle term destination? Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
The previous patch has a bug. It incorrectly complains about no layers being selected. This patch works better. marco On Sun, Jan 2, 2011 at 11:40 PM, Marco Mattila marco...@gmail.com wrote: Hi, During previous discussions about subtracting masks from silkscreen layers when plotting gerbers, Dick mentioned that saving plot settings is not quite up to date. Currently some plot settings to be saved/loaded are defined in pcbnew_config.cpp. However, that requires that corresponding variables in the PCB_PLOT_PARAMS class are public. My proposal is to let the class itself take care of saving/loading the settings. That's mainly what the attached patch changes. In addition, all plot dialog settings should now be included. The wxConfig key names are also edited to start with Plot, and layer selections are combined into a single bit mask. Moreover, some global variables in pcbplot.cpp have been moved into the class, too. g_PcbPlotOptions itself is still global. However, I think that it could be moved to be a member of the WinEDA_BasePcbFrame. That way it should be accessible where needed without being global. And if necessary, plot settings could still be loaded only once when pcbnew starts (now they are loaded every time the plot dialog constructor is called). In addition drill file generation settings could be included into PCB_PLOT_PARAMS. I can continue working on this if my approach sounds reasonable. marco I would say the patch looks like an improvement worth committing. Thanks. If anyone has concerns they should voice them. This little bit here is goofy though, for a function comment: PCB_PLOT_PARAMS(); +/** + Function LoadSettings + loads plot settings from wxConfig system. + @param aConfig is a pointer to a wxConfig. + */ +voidLoadSettings( wxConfig* aConfig ); +/** + Function SaveSettings + saves current plot settings to wxConfig system. + @param aConfig is a pointer to a wxConfig. + */ +void In the future, please leave a space between these like the coding standard says, and fill in the vertical stars. My vote is to commit. Dick ___ 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
Re: [Kicad-developers] [PATCH] Saving plot dialog settings
The previous patch has a bug. It incorrectly complains about no layers being selected. This patch works better. marco On Sun, Jan 2, 2011 at 11:40 PM, Marco Mattila marco...@gmail.com wrote: Hi, During previous discussions about subtracting masks from silkscreen layers when plotting gerbers, Dick mentioned that saving plot settings is not quite up to date. Currently some plot settings to be saved/loaded are defined in pcbnew_config.cpp. However, that requires that corresponding variables in the PCB_PLOT_PARAMS class are public. My proposal is to let the class itself take care of saving/loading the settings. That's mainly what the attached patch changes. In addition, all plot dialog settings should now be included. The wxConfig key names are also edited to start with Plot, and layer selections are combined into a single bit mask. Moreover, some global variables in pcbplot.cpp have been moved into the class, too. g_PcbPlotOptions itself is still global. However, I think that it could be moved to be a member of the WinEDA_BasePcbFrame. That way it should be accessible where needed without being global. And if necessary, plot settings could still be loaded only once when pcbnew starts (now they are loaded every time the plot dialog constructor is called). In addition drill file generation settings could be included into PCB_PLOT_PARAMS. I can continue working on this if my approach sounds reasonable. marco === modified file 'pcbnew/pcbframe.cpp' --- pcbnew/pcbframe.cpp 2010-12-21 15:13:09 + +++ pcbnew/pcbframe.cpp 2011-01-01 10:25:26 + @@ -56,7 +56,7 @@ extern int g_DrawDefaultLineThickness; // Keys used in read/write config -#define OPTKEY_DEFAULT_LINEWIDTH_VALUE wxT( PlotLineWidth ) +#define OPTKEY_PLOT_DEFAULT_LINEWIDTH_VALUE wxT( PlotDefaultLineWidth ) #define PCB_SHOW_FULL_RATSNET_OPT wxT( PcbFulRatsnest ) #define PCB_MAGNETIC_PADS_OPT wxT( PcbMagPadOpt ) #define PCB_MAGNETIC_TRACKS_OPT wxT( PcbMagTrackOpt ) @@ -513,7 +513,7 @@ WinEDA_BasePcbFrame::LoadSettings(); long tmp; -config-Read( OPTKEY_DEFAULT_LINEWIDTH_VALUE, g_DrawDefaultLineThickness ); +config-Read( OPTKEY_PLOT_DEFAULT_LINEWIDTH_VALUE, g_DrawDefaultLineThickness ); config-Read( PCB_SHOW_FULL_RATSNET_OPT, tmp ); GetBoard()-SetElementVisibility(RATSNEST_VISIBLE, tmp); config-Read( PCB_MAGNETIC_PADS_OPT, g_MagneticPadOption ); @@ -539,7 +539,7 @@ wxRealPoint GridSize = GetScreen()-GetGridSize(); -config-Write( OPTKEY_DEFAULT_LINEWIDTH_VALUE, g_DrawDefaultLineThickness ); +config-Write( OPTKEY_PLOT_DEFAULT_LINEWIDTH_VALUE, g_DrawDefaultLineThickness ); long tmp = GetBoard()-IsElementVisible(RATSNEST_VISIBLE); config-Write( PCB_SHOW_FULL_RATSNET_OPT, tmp ); config-Write( PCB_MAGNETIC_PADS_OPT, (long) g_MagneticPadOption ); === modified file 'pcbnew/pcbnew_config.cpp' --- pcbnew/pcbnew_config.cpp 2010-12-11 18:40:39 + +++ pcbnew/pcbnew_config.cpp 2010-12-29 19:02:06 + @@ -398,34 +398,5 @@ g_Show_Module_Ratsnest, TRUE ) ); m_configSettings.push_back( new PARAM_CFG_BOOL( true, wxT( TwoSegT ), g_TwoSegmentTrackBuild, TRUE ) ); -// Plot options: -m_configSettings.push_back( new PARAM_CFG_INT( true, wxT( HPGLnum ), - g_PcbPlotOptions.m_HPGLPenNum, - 1, 1, 16 ) ); -m_configSettings.push_back( new PARAM_CFG_INT( true, wxT( HPGdiam ), - g_PcbPlotOptions.m_HPGLPenDiam, - 15, 0, 100 ) ); -m_configSettings.push_back( new PARAM_CFG_INT( true, wxT( HPGLSpd ), - g_PcbPlotOptions.m_HPGLPenSpeed, - 20, 0, 1000 ) ); -m_configSettings.push_back( new PARAM_CFG_INT( true, wxT( HPGLrec ), - g_PcbPlotOptions.m_HPGLPenOvr, - 2, 0, 0x100 ) ); -m_configSettings.push_back( new PARAM_CFG_INT( true, wxT( PlotOutputFormat ), -g_PcbPlotOptions.m_PlotFormat, PLOT_FORMAT_GERBER ) ); -m_configSettings.push_back( new PARAM_CFG_BOOL( true, wxT( EdgeLayerGerberOpt ), -g_PcbPlotOptions.m_ExcludeEdgeLayer, true ) ); -m_configSettings.push_back( new PARAM_CFG_BOOL( true, wxT( SubstractMasktoSilk ), -g_PcbPlotOptions.m_SubtractMaskFromSilk, false ) ); -m_configSettings.push_back( new PARAM_CFG_BOOL( true, wxT( PlotPadsOnSilkscreen ), -g_PcbPlotOptions.m_PlotPadsOnSilkLayer, false ) ); -m_configSettings.push_back( new