kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

Personally I agree with your assessment, and I like the looks of some of what
you're doing, though I only had a chance to give it a quick glance. Perhaps you
could get in touch directly with Tom Włostowski? This is pretty much 'his' code
for the time being.

On Mon, Mar 07, 2016 at 10:59:10PM +1300, Chris Gagneraud wrote:
> Hi there,
> 
> I stumbled across the FOSDEM 2015 presentation recently, and was
> really impressed by the work done on the router. Near the last slides,
> something caught my attention:
>  - Written in C++ / Boost (optional, unordered)
>  - Separate data model.
>  - No UI library dependencies.
>  - Can be integrated into other F/OSS EDA tools
>    without much hassle.
> 
> It all sounded so great to me that I decided to have a roll and see
> how I could use PNS within my own projects.
> 
> To my surprises, dependecies were a bit heavier than expected:
>  - Boost (OK)
>  - Geom/math kernel (OK)
>  - Wx widgets (not too good)
>  - pcbnew/kicad (ouch!)
> 
> I knew there was a dependency on boost, I was not surprised to see a
> dependency on a tailored made math and geometry library, the wxWidget
> dependency didn't surprise me (knowing it is the toolkit used by
> KiCad), what worried me was the dependency on pcbnew itself.
> 
> The problem was quite simple to spot, yet not easy to fix:
>  - PNS_ROUTER is a god object, it knows everything, and everyone
>    relies on it.
>  - PNS_ROUTER is full of Wx and KiCad stuff
>  - The whole PNS core (NODE, INDEX, ITEM, ALGORITHM) depend on
>    PNS_ROUTER
>  - As a consequence the whole PNS core depends on Wx and KiCad.
>  - And anyway, part of PNS core directly depends on KiCad and
>    WxWidgets (like the "God forgive me doing this..." and the
>    diff-pair hacks. ;))
> 
> To fix this, I used a step by step refactoring approach (hence the
> hight amount of commits):
>  - Remove non-real dependencies on Wx and KiCad (unnecesary includes
>    files, unnecessary forward declarations)
>  - Move all KiCad dependend responsabilities from PNS_ALGORITHM,
>    PNS_NODE, PNS_INDEX, PNS_ITEM to PNS_ROUTER, This is about making
>    the core relying solely on the router to handle KiCad specific
>    stuff.
>  - Split PNS' router into a pure interface PNS_ROUTER_INTERFACE and
>    it's KiCad's implementation: PNS_ROUTER.
>  - Stuff PNS_ROUTER_INTERFACE with the strict minimum functions needed
>    to get the PNS core building (Node, Index, Algorithm, ...).
>  - Stuff PNS_ROUTER with everything else (PNS_ROUTER being now
>    unreachable from PNS core).
> 
> At that point the PNS core objects use a pointer to a
> PNS_ROUTER_INTERFACE and doesn't contains (almost) any KiCad
> dependencies (some exceptions were made for stuff I didn't fully
> understand at that time).
> 
> From there, the game could start:
>  - Do refactoring by grouping PNS_ROUTER_INTERFACE functions in
>  logical group, by either creating a new class to handle each group,
>  or modify existing ones. Examples include:
>    - PNS_CLEARANCE_RESOLVER (nee PNS_CLEARANCE_FUNC)
>    - PNS_NET_RESOLVER (Deal only with paired nets for now)
>    - PNS_DEBUG_DECORATOR (used solely by placers)
>  - Make ALGORITHM objects relies on world instead of router. The
>    router is still there, but now it actively assign the solvers to
>    the world NODE, and the algorithm access these solvers through the
>    world instead of through the router. Same story goes with SETTINGS.
>  - Commit, hack, rebase, hack, amend, hack, repeat forever until
>    brain's cooked up.
>  - Once the PNS_ROUTER_INTERFACE is empty, removes any reference to it
>    from PNS core.
> 
> At that point PNS_ROUTER looks more like a controller than a god
> object, and no PNS core objects depends on it. Cherry on the cake, all
> KiCad dependencies are gone, and only wxString, translation macro
> (underscore-brackets) and wxTimer.
> 
> And that's where I stopped, I think I've already made big enough
> modifications before i can keep going. I need to discuss this further
> with you guys, to see if you are interested in this kind of work, to
> organise a code review, get a feeling, ...
> 
> You can see that as an Hackaton session, I did all this in 4 days
> (part time, sorry!).
> 
> I broke a few things in the process, but I do think that they should
> be easy to fix (especially through code review, as my commits are
> organised in logical chunks).
> 
> I think the PNS router is an awesome framework, I like it's approach:
>  - Spatial indexing of the PCB features (with layer and joint
>    awareness)
>  - Clever branch/merge of indexes.
>  - Algorithm to handle PCB modifications
>  - Separate data model
>  - No UI dependencies
> 
> But my feeling is there's still need for refactoring. For example:
> 
> On the inner side:
>  - Completely get rid of wxDeps (string, translation and timers)
>  - Make ALGO straight forward to use: setWorld(), configure(),
> execute(), getResult()
>  - Properly fix coupling of KiCad/PNS via and layer(set). (my solution
>    look more like a gross hack for now).
>  - Remove PLACER dependency from items (why pns_meander.cpp needs
>    pns_meander_placer_base.h?)
>  - Remove NODE dependency from some items (line, meander, via)
>  - Remove NODE dependency from SETTINGS
>  - Remove NODE dependency from OPTIMISER and TOPOLOGY (I think it's
>    doable by making them rely on a PNS_INDEX interface, provided by
>    NODE with a proxy/facade design pattern.)
>  - Only router and algorithm should have access to NODE
>  - ITEM, TOPOLOGY and OPTIMISER should relies only on INDEX (INDEX is
>    the spacial specialist, NODE is the branch/merge specialist)
> 
> On the outer side:
>  - Split the remaining code of the PNS_ROUTER into an implementation
>    independent controller (the real PNS routing public interface) and
>    it's KiCad implementation/usage.
> 
> 
> 
> 
> That's it! I hope you will welcome my ideas, and I'm looking forward
> to hear your feedback (positive or negative). I am definitely willing
> to re-work my patches until it is acceptable to the PNS maintainers.
> 
> I have 26 commits, they are available on GitHub at
> https://github.com/chgans/kicad/.
> 
> What would be the next step to get this RFC going on?
> For convenience, I could open a pull request against GitHuib official
> mirror. I know you guys use bazar, but at this point I just want my
> hacks^W work to be commented on, a GitHub pull request could allow
> that. Or I could send all the patch on the mailing list as they do
> with other open source projects, I don't mind that either.
> 
> Best regards,
> Chris
> 
> _______________________________________________
> 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

-- 
Chris