Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-13 Thread Wayne Stambaugh
Jon, I misunderstood your original intent. I don't think cluttering the board layer enums with all of the virtual layer and schematic layer enums is a good idea. It just seems like overkill to me. I thought you were going to create a separate enum for virtual board layers. You could always sta

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-13 Thread Jon Evans
Hi Wayne, I understand this might seem like too big a change. Here is what I was thinking when I thought that combining everything would be a good solution. - If there is more than one enum, then functions that consume data from more than one app (i.e. things in common/GAL) have to cast to int, s

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-13 Thread Maciej Suminski
Hi, How about emulating enum inheritance [1]? I suppose it would be the cleanest solution allowing us to clearly specify what kind of layer is expected for certain methods. This is even better kind of type checking. Cheers, Orson 1. https://www.codeproject.com/kb/cpp/inheritenum.aspx On 03/13/2

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-13 Thread Jon Evans
Hi Orson, It's an interesting idea, I will have to look at it more. But, doesn't this still allow the programmer to accidentally overlap two enum values? I can add checks to prevent this from happening elsewhere in the code, but it seems less clean to me. Best, -Jon On Mon, Mar 13, 2017 at 1:5

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-14 Thread Jon Evans
Hi Orson, Wayne, I looked at the "enum inheritance" thing some more and I don't think it would be a good solution for our case. This technique lets you extend enum A with enum B, and have functions f(A) and f(A or B), and you could continue making larger enums that contained smaller ones. But, if

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-14 Thread Wayne Stambaugh
On 3/14/2017 10:08 AM, Jon Evans wrote: > Hi Orson, Wayne, > > I looked at the "enum inheritance" thing some more and I don't think it > would be a good solution for our case. > > This technique lets you extend enum A with enum B, and have functions > f(A) and f(A or B), and you could continue ma

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-14 Thread John Beard
Hi Jon, Protocol Buffers has the same problem. Messages have a unique number for each field, but extensions to messages don't know about "siblings". A common thing [1] to so is reserve IDs up to 1000 for the base message, and then messages start at offsets 1000, 2000, etc, based on some in-house s

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-14 Thread Jon Evans
Hi John, that's basically what my first patch did, but inside one enum. Hi Wayne, thanks for elaborating more, I see your point. I am still not sure there is benefit to adding some class to handle enum inheritance. I think it would work fine to just chain multiple enums together, as was done befo

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-14 Thread Wayne Stambaugh
Hey Jon, This is better than the giant enum concept and I'm willing to accept this. It still lacks the type safety of the enum inheritance solution. I still see ints being cast to enums and enum bounds checking so this is less than ideal. I would prefer to see some additional testing so if any o

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-14 Thread Jon Evans
Hi Wayne, Please keep in mind that anywhere there is currently int casting and bounds checking that deals with colors, will be replaced with type-safe methods in an upcoming patch from me. For those places not dealing with colors, I will see if it can be improved in a future patch. Thanks, Jon

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-15 Thread Lorenzo Marcantonio
On Tue, Mar 14, 2017 at 07:05:59PM -0400, Wayne Stambaugh wrote: > Hey Jon, > > This is better than the giant enum concept and I'm willing to accept > this. It still lacks the type safety of the enum inheritance solution. Bad memories with layers enums :D funny thing when I proposed the layer_id

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-15 Thread Lorenzo Marcantonio
On Tue, Mar 14, 2017 at 07:09:30PM -0400, Jon Evans wrote: > Hi Wayne, > > Please keep in mind that anywhere there is currently int casting and bounds > checking that deals with colors, will be replaced with type-safe methods in > an upcoming patch from me. For those places not dealing with color

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-15 Thread Jon Evans
Hi Lorenzo, I think this is possible. I will look in to it. -Jon On Wed, Mar 15, 2017 at 3:05 AM, Lorenzo Marcantonio wrote: > On Tue, Mar 14, 2017 at 07:09:30PM -0400, Jon Evans wrote: > > Hi Wayne, > > > > Please keep in mind that anywhere there is currently int casting and > bounds > > che

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-16 Thread Jon Evans
Bump -- does anyone have time to look at this and report back if there are any issues? I'd like to get it merged so that I can feel confident about doing more work on top of these changes. Thanks, Jon On Tue, Mar 14, 2017 at 6:05 PM, Wayne Stambaugh wrote: > Hey Jon, > > This is better than th

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-21 Thread Wayne Stambaugh
Jon, I just attempted to apply your patch and it no longer applies cleanly. Please rebase it when you get a chance. Thanks, Wayne On 3/16/2017 10:14 AM, Jon Evans wrote: > Bump -- does anyone have time to look at this and report back if there > are any issues? I'd like to get it merged so that

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-22 Thread Maciej Sumiński
I have briefly tested the patch, no issues found. I have no objections to the changes, but I will leave the final decision to Wayne. Regards, Orson On 03/22/2017 03:51 AM, Jon Evans wrote: > Hi Wayne, new patch attached. > > Thanks, > Jon > > On Tue, Mar 21, 2017 at 9:28 AM, Wayne Stambaugh >

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-24 Thread Jon Evans
Hi Wayne, Any feedback on this? Thanks, Jon On Wed, Mar 22, 2017 at 6:15 AM, Maciej Sumiński wrote: > I have briefly tested the patch, no issues found. I have no objections > to the changes, but I will leave the final decision to Wayne. > > Regards, > Orson > > On 03/22/2017 03:51 AM, Jon Evan

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-28 Thread Wayne Stambaugh
John, Please rebase your patch when you get a chance. I just attempted to apply it so I could do some testing and it no longer applies. Sorry for the headaches but I'm really busy. I will apply it as soon as I get it and do some testing. Thanks, Wayne On 3/24/2017 8:54 AM, Jon Evans wrote: >

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-28 Thread Wayne Stambaugh
Hey Jon, I'm getting the following build error (there may be more but this is where my build stopped): C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/router/router_tool.cpp: In member function 'int ROUTER_TOOL::onViaCommand(const TOOL_EVENT&)': C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/r

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-30 Thread Wayne Stambaugh
Hey Jon, I finally found some time to test and review this patch and it seems to work just fine. My main concern is placing the schematic, gerber viewer, and board layer IDs and their associated functions, operators, and classes into a single file which may make creating separate board and schema

Re: [Kicad-developers] [PATCH] Refactor LAYER_ID to be the one and only layer definition

2017-03-30 Thread Wayne Stambaugh
Jon, I stand corrected. I found an issue with your patch. The layer count in the schematic is wrong. Before: EELAYER 26 0 After EELAYER 157 0 I know that this really isn't used anywhere but users who use a VCS will complain about the unnecessary delta. Wayne On 3/30/2017 8:59 AM, Wayne St