kicad-developers team mailing list archive

[Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>]

On 15/11/17 18:38, Wayne Stambaugh wrote:
> It looks like the PCB_COLLECTOR objects are not swigged yet but that
> would be the way to do it.  Properly swigged, there would be nothing
> preventing you from collecting all objects connected to an net or any
> other collection criteria for that matter.  That's what the collector
> object is for.  Let's not reinvent the wheel.

Hi Wayne,

I'm fine with swigging the COLLECTORS, but they aren't in my opinion the
simplest way to execute complex queries on a board. I would use
iterators instead. The DLIST_ITERS in the BOARD class that hide the
DLISTs behind a C++-style iterator can be extended with filter methods,
e.g.:

// Iterates all tracks in net 1 and on Top layer.

for ( auto track : board->Tracks().InNet(1).OnLayer( LAYER_TOP ) ) {...}




Cheers,
Tom





> 
> On 11/15/2017 10:59 AM, Tomasz Wlostowski wrote:
>> On 15/11/17 15:37, miles mccoo wrote:
>>> Hello,
>>>
>> Hello Miles,
>>
>> Thanks for the patch. My comments below:
>>
>>>
>>> A couple code review type observations on the existing code.
>>>
>>>  1. The new connectivity stuff is in connectivity.h. Perhaps it should
>>>     be called class_connectivity.h to match the naming of other files?
>>>     better yet class_connectivity_data.h
>> Nope. Ideally, we should have one class-one file mapping, we're not
>> there yet. Even the files that are prefixed with class_* don't follow
>> the rule strictly (e.g. CLASS_BOARD). I'd rather rename it to
>> connectivity_data.h to match the inner class name.
>>
>>
>>>  2. The initial commit comment of connectivity.h should says this:
>>>     "New connectivity algorithm."
>>>     It would have been helpful to have more. Presumably, there's a
>>>     design document somewhere describing the change. Or perhaps and
>>>     email thread? Could have saved some hunting.
>>
>>>  3. The API that I'm exposing in python is this one:
>>>     const std::list<BOARD_CONNECTED_ITEM*> GetNetItems( int aNetCode,
>>>                                            const KICAD_T aTypes[] ) const;
>>>
>>>     And is used like this:
>>>     const KICAD_T types[] = { PCB_PAD_T, PCB_ZONE_AREA_T, EOT };
>>>     auto netItems = m_connectivity->GetNetItems( i, types );
>>>
>>>     why not just use a std::vector? Adding the EOT is another thing
>>>     folks have to remember to include.
>> I'm OK with that. Feel free to add convenience wrappers for Python (e.g.
>> BOARD::GetNetItems, etc.) in the swig interface files.
>>
>> Concerning the use of std::list - it's there probably because the
>> previous ratsnest code (RN_DATA) returned a std::list. I don't mind
>> changing the function to return std::vector. Could you update your patch
>> when it's done?
>>
>>>  4. m_connectivity is a public member of connectivity_data and used all
>>>     over the place. My head is ringing with the voices of my college TAs
>>>     about access methods. 
>> Nope, it's not. I don't know what version of Kicad code you looked at,
>> but certainly in the current master m_connectivity is private.
>>
>> Tom
>>
>> _______________________________________________
>> 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
>>
> 
> _______________________________________________
> 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
>