kicad-developers team mailing list archive

[Andrew Lutsenko <anlutsenko@xxxxxxxxx>]

Hi Seth,

That makes sense. I will keep working on this feature and will ping this
thread again once user configuration is implemented.

Thanks,
Andrew

On Fri, Aug 17, 2018 at 10:03 AM Seth Hillbrand <seth@xxxxxxxxxxxxx> wrote:

> Hi Andrew-
>
> I like the patch idea and your implementation approach is good.
>
> The coding style policy is located at
> https://kicad-source-mirror.readthedocs.io/en/stable/Documentation/development/coding-style-policy/
> We're not totally consistent but we try to ensure any new code follows it
> and clean up the old code as we go.
>
> On the errors, please don't throw an error to the user.  It may be useful
> to insert a wxLog() call that we could utilize in the future but that is
> ignored for now.
>
> I'm opposed to merging patches that are partially complete.  And I would
> consider the lack of user control over this feature problematic.  This is
> not a judgement on the patch as you've implemented it.  I really like the
> functionality and think KiCad users will appreciate it as well.  However, a
> partially implemented feature creates the opportunity for problems down the
> road that will distract developer time from the other tasks they are
> working on.  If you do not have the time to fully implement user control
> over this feature (I completely understand competing time pressures), you
> may consider opening a bug report and attaching your patch there for
> interested future developers.  Squashing the patchset for review is also a
> good idea.
>
> Overall this looks really promising.
>
> Best-
> Seth
>
> Am Do., 16. Aug. 2018 um 23:02 Uhr schrieb Andrew Lutsenko <
> anlutsenko@xxxxxxxxx>:
>
>> Hi Seth
>>
>> I just checked out new preferences in pcbnew, looks much more organized
>> than before.
>> I totally can add a new tab there "pcbnew->Action plugins" and list the
>> plugins there with option
>> to remove toolbar icon. But that is a non-negligible amount of work. Will
>> you hold off on merging
>> my current patches until I implement that too?
>> By default plugins will not show any buttons on toolbar, plugin writers
>> will have to explicitly update
>> their plugins and provide an icon for them to show up so you will not run
>> into an issue with full
>> toolbar for a while. See my screenshot in second email of the chain, it
>> has 4 plugins but only
>> 2 of them register with an icon and toolbar button.
>>
>> > headers get 1 space between function defs
>> I tried to follow existing style in each file and didn't notice that it's
>> not consistent across different files.
>> action_plugin.h has two new lines between most functions but I can change
>> it to one.
>>
>> What do you think about throwing an error to user if icon failed to load?
>> Andrey Kuznetsov made a
>> point that user can't do anything about it anyway. I agree that asking
>> users to fix plugin icon declaration
>> is a bit much and developer will be able to see that icon didn't load
>> without the error too.
>>
>> Andrew
>>
>> On Thu, Aug 16, 2018 at 10:22 PM Seth Hillbrand <seth@xxxxxxxxxxxxx>
>> wrote:
>>
>>> Hi Andrew-
>>>
>>> I like the idea.  Aside from minor formatting (headers get 1 space
>>> between function defs, need a space before the if block), the patch looks
>>> good.
>>>
>>> However, I wouldn't want everything showing on my toolbar (speaking as
>>> someone who has 10 plugins installed, 5 of which get regular use).  I'd
>>> prefer the option to be configurable.  This should probably be in the
>>> preferences pane that Jeff has been re-working.
>>>
>>> -Seth
>>>
>>> Am Do., 16. Aug. 2018 um 22:11 Uhr schrieb Andrew Lutsenko <
>>> anlutsenko@xxxxxxxxx>:
>>>
>>>> Hi Clemens,
>>>>
>>>> See sample plugin attached. Extract it into kicad's
>>>> share/scripting/plugins folder.
>>>> One of other scanned directories that are documented in kicadplugins.i
>>>> <https://github.com/KiCad/kicad-source-mirror/blob/6fdc5972f8431b4d5831a32649e67bfe20d05de8/scripting/kicadplugins.i#L180> should
>>>> work too.
>>>>
>>>> Or are you asking to update docs in the repo?
>>>> Documentation/development/pcbnew-plugins.md seems like the right place.
>>>> I will update it once committers agree with the path I've chosen to
>>>> implement this.
>>>>
>>>>
>>>>
>>>> On Thu, Aug 16, 2018 at 4:48 AM Clemens Koller <cko@xxxxxxxxx> wrote:
>>>>
>>>>> Hello,  Andrew!
>>>>>
>>>>> I am somehow missing the Python example you are giving in your patch.
>>>>> Can you add this a simple example to the sources to get a basic/dummy
>>>>> skeleton working right from scratch?
>>>>>
>>>>> Regards,
>>>>>
>>>>> Clemens
>>>>>
>>>>>
>>>>> On 15/08/2018 11.31, Andrew Lutsenko wrote:
>>>>> > Hi KiCad devs,
>>>>> >
>>>>> > I am proposing an addition to plugin system.
>>>>> > Probably most will agree that menus suck. Toolbars suck less :)
>>>>> >
>>>>> > In my plugin I added a dirty hack to modify top toolbar from plugin
>>>>> init code to add a button
>>>>> > that calls plugins run() method. It is broken on linux X11 and is
>>>>> not a sustainable way others
>>>>> > can add buttons in their plugins. But having a button was quite
>>>>> popular among users so I
>>>>> > decided to implement this functionality directly in pcbnew.
>>>>> >
>>>>> > I introduced one more field plugin writers can define in defaults()
>>>>> that contains path to png icon
>>>>> > and if that string is not empty, pcbnew will attempt to load that
>>>>> icon and add a button to top
>>>>> > toolbar with action that calls the same run() method. I traced in
>>>>> code how plugin action menu
>>>>> > is generated and added similar logic for buttons.
>>>>> >
>>>>> > Here is how the result looks like:
>>>>> >
>>>>> > https://i.imgur.com/f3xg1FE.gif
>>>>> >
>>>>> > Sample dummy plugin __init__.py code:
>>>>> >
>>>>> > import os
>>>>> > import pcbnew
>>>>> > import wx
>>>>> >
>>>>> > class Plugin1(pcbnew.ActionPlugin):
>>>>> >
>>>>> >     def defaults(self):
>>>>> >         self.name <http://self.name> = "Dummy Plugin 1"
>>>>> >         self.category = "Read PCB"
>>>>> >         self.description = ""
>>>>> >         self.icon_file_name =
>>>>> os.path.join(os.path.dirname(__file__), 'icon.png')
>>>>> >
>>>>> >     def Run(self):
>>>>> >         wx.MessageBox("Plugin 1")
>>>>> >
>>>>> > Plugin1().register()
>>>>> >
>>>>> > It's as simple as that.
>>>>> >
>>>>> > The patch is attached. It probably needs some error checking but
>>>>> seems to be working great.
>>>>> > Tested in win64 so far.
>>>>> > I'm open to suggestions on how to get it to good state, I will also
>>>>> test on linux asap.
>>>>> >
>>>>> > Regards,
>>>>> > Andrew
>>>>> >
>>>>> > _______________________________________________
>>>>> > 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
>>>>>
>>>> _______________________________________________
>>>> 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
>>>>
>>>