pcb

Added possibility to edit pin/pad numbers (New feature)

Bug #1510313 reported by Milan Prochac
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pcb
In Progress
Medium
Bert Timmerman

Bug Description

Attached patch adds possibility to edit pin/pad numbers interactively when option "Pinout shows numbers" is checked.

It also adds customization of user entry dialog for pin/pad name/value, and also for element name/value/description - the correct name of field is displayed (translations need to be updated).

New feature shares the undo misbehavior with element name/value/description change - the undo is applied to currently displayed names, not to actual changed name, i.e - if element name is changed, the view is switched to dosplay values, undo reverts the value, not the name. This will be addressed by separate patch.

Revision history for this message
Milan Prochac (milan-x) wrote :
Changed in pcb:
status: New → Triaged
milestone: none → next-feature-release
importance: Undecided → Medium
Changed in pcb:
status: Triaged → In Progress
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Milan,

I just tested this patch: works as advertised.

I have yet to check whether changes show up in the pcb file after saving.

And if working in the lesstif GUI.

In the mean time the patch is pushed to topic branch "LP1510313" in the upstream pcb repository, for others to test.

Thanks for the patch and kind regards,

Bert Timmerman.

Changed in pcb:
assignee: nobody → Bert Timmerman (bert-timmerman)
Revision history for this message
Milan Prochac (milan-x) wrote :

Attached patch for UNDO misbehavior mentioned before.

Patch fixes the UNDO behavior, when UNDO applies the reverse change to incorrect text - reverse change is applied to currently displayed text.

The patch adds the "text index" into the undo buffer structure; the index is used by UNDO operation to revert correct text.

The fix applies to pin/pad number/name and to element description/name/value.

The patch should be applied over the previous patch attached to this bug report.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Milan,

I just pushed patch 0002 to the upstream topic branch for testing.

Thanks and kind regards,

Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Milan,

Just tested patches 0001 and 0002 on both the gtk and lesstif GUIs and they seem to work as advertised.

Kind regards,

Bert Timmerman.

Changed in pcb:
milestone: next-feature-release → next-bug-release
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :
Download full text (3.9 KiB)

Hi,

Just rebased the topic branch (home/bert/LP1510313) to "master HEAD" and this breaks the renumbering of blocks (selected elements) and buffer in renumber.c.

Providing a dummy idx "NAMEONPCB_INDEX" in lines 104, 106 and 169 (par example as in action.c: line 3539) creates a segfault.

Kind regards,

Bert Timmerman.

gdb spew follows:

Program received signal SIGSEGV, Segmentation fault.
renumber_block (argc=2, argv=0x86beeb8, x=0, y=0) at renumber.c:91
91 for (refdes_split = cp = old_ref; *cp; cp++)
(gdb) bt
#0 renumber_block (argc=2, argv=0x86beeb8, x=0, y=0) at renumber.c:91
#1 0x080d90e1 in hid_actionv (name=0x86d8f40 "RenumberBlock", argc=2, argv=0x86beeb8) at hid/common/actions.c:243
#2 0x080d946a in hid_parse_actionstring (rstr=0x86a8818 "RenumberBlock(100,200)", require_parens=0 '\000')
    at hid/common/actions.c:327
#3 0x080e49e0 in command_entry_activate_cb (widget=0x86c0910 [GtkEntry], data=0x0) at hid/gtk/gui-command-window.c:211
#4 0x05a345d4 in IA__g_cclosure_marshal_VOID__VOID (closure=0x87d0fb8, return_value=0xbfffe60c, n_param_values=1,
    param_values=0x86bef08, invocation_hint=0xbfffe510, marshal_data=0x0) at gmarshal.c:77
#5 0x05a266e3 in IA__g_closure_invoke (closure=0x87d0fb8, return_value=0xbfffe60c, n_param_values=1, param_values=0x86bef08,
    invocation_hint=0xbfffe510) at gclosure.c:767
#6 0x05a3b255 in signal_emit_unlocked_R (node=0x86c0068, detail=0, instance=0x86c0910, emission_return=0xbfffe60c,
    instance_and_params=0x86bef08) at gsignal.c:3248
#7 0x06cd017a in gtk_binding_entry_activate (entry=<value optimized out>, object=0x86c0910 [GtkEntry]) at gtkbindings.c:537
#8 0x06cd076f in binding_match_activate (pspec_list=<value optimized out>, object=0x86c0910 [GtkEntry], path_length=8, path=
    0x86c2dd8 "GtkEntry", path_reversed=0x83555f8 "yrtnEktG", unbound=0xbfffe6b0) at gtkbindings.c:1123
#9 0x06cd0a0a in gtk_bindings_activate_list (object=0x86c0910 [GtkEntry], entries=<value optimized out>, is_release=0)
    at gtkbindings.c:1268
#10 0x06cd0b5b in IA__gtk_bindings_activate_event (object=0x86c0910 [GtkEntry], event=0x87cfad8) at gtkbindings.c:1363
#11 0x06d25393 in gtk_entry_key_press (widget=0x86c0910 [GtkEntry], event=0x87cfad8) at gtkentry.c:4120
#12 0x06db09a8 in _gtk_marshal_BOOLEAN__BOXED (closure=0x81cb998, return_value=0xbfffe904, n_param_values=2, param_values=
    0x87778c8, invocation_hint=0xbfffe8f0, marshal_data=0x6d252e0) at gtkmarshalers.c:84
#13 0x05a24d1a in g_type_class_meta_marshal (closure=0x81cb998, return_value=0xbfffe904, n_param_values=2, param_values=
    0x87778c8, invocation_hint=0xbfffe8f0, marshal_data=0xcc) at gclosure.c:878
#14 0x05a26608 in IA__g_closure_invoke (closure=0x81cb998, return_value=0xbfffe904, n_param_values=2, param_values=0x87778c8,
    invocation_hint=0xbfffe8f0) at gclosure.c:767
#15 0x05a3aea6 in signal_emit_unlocked_R (node=0x81cb910, detail=0, instance=0x86c0910, emission_return=0xbfffea4c,
    instance_and_params=0x87778c8) at gsignal.c:3286
#16 0x05a3c4d5 in IA__g_signal_emit_valist (instance=0x86c0910, signal_id=42, detail=0, var_args=
    0xbfffeab0 "\334\352\377\277\070\060\034\b\001") at gsignal.c:2991
#17 0x05a3caf7 in IA__g_s...

Read more...

Revision history for this message
Milan Prochac (milan-x) wrote :

I was able to generate segfaults only and only when refdes was missing on one of element; it applies for both RenumberBlock and RenumberBuffer

There are missing refdes validity checks at lines 91 and 158 in renumber.c

With all refdeses valid, all works fine - no crashes, all is renumbered as expected. I found problem with RenumberBuffer, where new texts are drawn on screen - this is possibly because function Draw() is called unconditionally from ChangeObjectName - but this is also issue in original code.

I tested both issues with original code (without my patch) - the behaviour (crashes for elements without refdes and texts drawn on screen as well) is identical.

Kind regards
Milan

Changed in pcb:
milestone: next-bug-release → next-feature-release
Revision history for this message
Peter Clifton (pcjc2) wrote :

Hi Milan,

Regarding the crash you found, a test-case for that would be very welcome! (Obviously, a fix would too!).

Peter

Revision history for this message
Peter Clifton (pcjc2) wrote :

Bert - haven't thoroughly reviewed patch contents, but would probably suggest a squash of the two patches before applying, given their descriptions..

A quick skim suggests some coding style improvements may be possible, in particular the first patch. Perhaps one for the next code-sprint?

Peter

Revision history for this message
Milan Prochac (milan-x) wrote :

Well, here is the patch to fix the RenumberBlock and RenumberBuffer crashes.

Just added the condition - elements without refdes are skipped.

Regards
Milan

Revision history for this message
Milan Prochac (milan-x) wrote :

And the small test board

Changed in pcb:
milestone: pcb-4.1.0 → pcb-4.2.0
Changed in pcb:
milestone: pcb-4.2.0 → future-feature-release
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.