pcb

[BOM HID] Attributes

Bug #1808733 reported by Chad Parker on 2018-12-17
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pcb
Wishlist
Bert Timmerman

Bug Description

The included patch was submitted by DJ. It allows the BOM hid to output element attributes. Use case is generating digi-key orders.

Chad Parker (parker-charles) wrote :
Bert Timmerman (bert-timmerman) wrote :

Hi Charles,

Tries to apply in a topic branch on current master.

2 out of 10 hunks FAILED ... now trying to patch up by hand.

Kind regards,

Bert Timmerman.

Bert Timmerman (bert-timmerman) wrote :

Hi,

OK, ready for review.

This feature could use some more elaboration in the User manual ... constraints/rules for the file format and/or content and whether a path is allowed.

Kind regards,

Bert Timmerman

Changed in pcb:
assignee: nobody → Bert Timmerman (bert-timmerman)
status: New → In Progress
Bert Timmerman (bert-timmerman) wrote :
Bert Timmerman (bert-timmerman) wrote :
Changed in pcb:
milestone: future-feature-release → pcb-4.2.0
Chad Parker (parker-charles) wrote :

Can you add this to the test suite?

Bert Timmerman (bert-timmerman) wrote :

Hi,

Further testing in a test suite gives me the impression that the "--attrs" file needs to have an absolute path, or that the default filename "attribs" needs to reside in the directory from which the pcb application is launched.

To be continued.

Kind regards,

Bert Timmerman.

Yes. It has to be added to the list of input files so that it gets copied
to the working directory. I think it's the second field of the line.
However, now that I'm thinking about it, I'm not sure if the test script
will recognize it since it doesn't have a known extension... None of the
other tests have ancillary files. I'll try to look at it this afternoon.

On Sat, Jan 5, 2019, 16:25 Bert Timmerman <<email address hidden> wrote:

> Hi,
>
> Further testing in a test suite gives me the impression that the "--
> attrs" file needs to have an absolute path, or that the default filename
> "attribs" needs to reside in the directory from which the pcb
> application is launched.
>
> To be continued.
>
> Kind regards,
>
> Bert Timmerman.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1808733
>
> Title:
> [BOM HID] Attributes
>
> Status in pcb:
> In Progress
>
> Bug description:
> The included patch was submitted by DJ. It allows the BOM hid to
> output element attributes. Use case is generating digi-key orders.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/pcb/+bug/1808733/+subscriptions
>

Chad Parker (parker-charles) wrote :

I pushed a commit to LP1808733.

It adds the .attrs suffix to the known input files filter, and adds a test for the attributes output.

Hi,

I changed the error handling of input files without an extension in tests/run_tests.sh and just print a info message before continuing.

This may prove necessary for allowing other input file types such as netlists or cad interchange files from other CAD applications in the future ;-)

Additional elaboration about the requirement of the "attribs" file in the current working directory (from which pcb was invoked) in the User Manual may be needed.

Please check and test topic branch "home/bert/LP1808733".

Kind regards,

Bert Timmerman.

Chad Parker (parker-charles) wrote :

Two things:

1. I think that naming the file "attribs" is too generic. It needs to be
named something that will associate it with the test it's used for.
2. The message on line 840 is not really correct. The * wildcard catches
everything, not just things without a file extension. So, the message
should read something like "WARNING: Not processing input file of
unidentified type: $f"

Otherwise, it looks good.

On Mon, Jan 7, 2019 at 3:30 PM Bert Timmerman <email address hidden>
wrote:

> Hi,
>
> I changed the error handling of input files without an extension in
> tests/run_tests.sh and just print a info message before continuing.
>
> This may prove necessary for allowing other input file types such as
> netlists or cad interchange files from other CAD applications in the
> future ;-)
>
> Additional elaboration about the requirement of the "attribs" file in
> the current working directory (from which pcb was invoked) in the User
> Manual may be needed.
>
> Please check and test topic branch "home/bert/LP1808733".
>
> Kind regards,
>
> Bert Timmerman.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1808733
>
> Title:
> [BOM HID] Attributes
>
> Status in pcb:
> In Progress
>
> Bug description:
> The included patch was submitted by DJ. It allows the BOM hid to
> output element attributes. Use case is generating digi-key orders.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/pcb/+bug/1808733/+subscriptions
>

Hi,

I removed the "exit -1" to avoid "not processing" a file without an extension in the filename.

We could keep this safety net of the wildcard "*" catching "everything" and exit 1, if we limit/force users to only use ".attrs" as an extension in the filename.

Kind regards,

Bert Timmerman.

Note: AFAICT, checking for an ".attrs" file name extension applies only for the test suite.The BOM HID happily accepts file names without an extension ;-)

Chad Parker (parker-charles) wrote :

Common users aren't adding files to the test suite, just sometimes running
it when they build from source. Since the HID doesn't care what the file
name is, we're not really placing any restrictions on users by forcing the
use of the attrs suffix.

I don't mind using the wildcard to let these files through, but I want the
warning message to be more accurate.

And I'd like the file name to somehow associate the file with the tests
it's used for. That's just good housekeeping.

On Tue, Jan 8, 2019 at 5:40 PM Bert Timmerman <email address hidden>
wrote:

> Note: AFAICT, checking for an ".attrs" file name extension applies only
> for the test suite.The BOM HID happily accepts file names without an
> extension ;-)
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1808733
>
> Title:
> [BOM HID] Attributes
>
> Status in pcb:
> In Progress
>
> Bug description:
> The included patch was submitted by DJ. It allows the BOM hid to
> output element attributes. Use case is generating digi-key orders.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/pcb/+bug/1808733/+subscriptions
>

Hi Charles,

Thanks for the feedback ;-)

I'm going to merge/cherry-pick this into master ... this evening.

Before that, I will have a second look at the "error" message associated with the wildcard and adjust as required.

Kind regards,

Bert Timmerman.

Changed in pcb:
status: In Progress → Fix Committed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers