yaqwsx / PcbDraw

Convert your KiCAD board into a nicely looking 2D drawing suitable for pinout diagrams
MIT License
1.13k stars 90 forks source link

"ValueError: Cannot parse '470' to resistance" (populate) #117

Closed dhaillant closed 1 year ago

dhaillant commented 1 year ago

Hello, I'm trying to use the res_band feature with populate. I get this error:

Traceback (most recent call last): File "/home/david/.local/bin/pcbdraw", line 8, in sys.exit(run()) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1128, in call return self.main(args, kwargs) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1053, in main rv = self.invoke(ctx) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1659, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1395, in invoke return ctx.invoke(self.callback, ctx.params) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 754, in invoke return __callback(args, kwargs) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/populate.py", line 300, in populate parsed_content = generate_images(parsed_content, board, prepare_params(header["params"]), File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/populate.py", line 193, in generate_images generate_image(boardfilename, x["side"], x["components"], File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/populate.py", line 212, in generate_image plot.main(args=plot_args) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1053, in main rv = self.invoke(ctx) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1395, in invoke return ctx.invoke(self.callback, ctx.params) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 754, in invoke return __callback(*args, **kwargs) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/ui.py", line 174, in plot image = plotter.plot() File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 1053, in plot plotter.render(self) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 812, in render plotter.walk_components(invert_side=False, callback=self._append_component) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 1084, in walk_components callback(lib, name, ref, value, pos) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 840, in _append_component ret = self._create_component(lib, name, ref, value) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 890, in _create_component self._apply_resistor_code(component_element, id_prefix, ref, value) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 912, in _apply_resistor_code res, tolerance = self._get_resistance_from_value(value) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/plot.py", line 947, in _get_resistance_from_value res = read_resistance(value_l[0]) File "/home/david/.local/lib/python3.8/site-packages/pcbdraw/unit.py", line 39, in read_resistance raise ValueError(f"Cannot parse '{value}' to resistance") ValueError: Cannot parse '470' to resistance

If I understand correctly, the value '470' is retrieved from the .kicad_pcb file. The script parses correctly an other value (100k) and generates correctly the corresponding picture.

Also, in this case I use the default resistor footprint. Not my custom one (see other issue #116 )

Thank you.

Electro707 commented 1 year ago

Can confirm this is a bug, and I can pin-point what is the cause of the crash.

yaqwsx commented 1 year ago

That Pcbdraw doesn't accept values without any marking is on purpose - all the schematics that I have ever worked with never used raw numbers to mark the resistance. Therefore, a raw number was considered an error that should be reported to the user, and they should review if there is an error or not.

@dhaillant Do you commonly use "ohm" values without any marking? Is it a common practice? If so, I will be happy to change the behavior.

Electro707 commented 1 year ago

@yaqwsx Even with the marking, for example 10Ohm, the function will still raise a ValueError the way it's currently implemented.

yaqwsx commented 1 year ago

10Ohm should be parsed, and that we should fix. At the moment, I am more interested in if 10 should be parsed or not.

Electro707 commented 1 year ago

I am of the personal opinion that it should, as most schematic do not bother to add the unit if the symbol is clearly a resistor.

dhaillant commented 1 year ago

Personally, for resistors, I use 470 without unit in my schematics, when there's no confusion possible (when the symbol is clearly a resistor).

I did some quick research and found many different ways among magazines, service manuals, open source schematics etc. Some use an R, others just numbers. And even, in some schematics, I found both styles (R seems to be added when the values are under 100 Ω)...

Now, I was wondering how the script handles values such as 4k7, 4.7k, 4.7R, 4R7, 0.47, 0R47, 4700k etc?

Thanks!

Electro707 commented 1 year ago

As currently implemented and with my fix (PR #118), here how the script handles the numbers you have:

4k7      ->      4007.0
4.7k     ->      4700.0
Failed for 4.7R
Failed for 4R7
0.47     ->      0.47
Failed for 0R47
4700k  ->      4700000.0
Electro707 commented 1 year ago

With my recent commit 04c0e01aea8307cb0df8250cf5e4b8ed7bcd3861, I was able to get the resistance parsing function to properly parse all the examples you gave:

4k7      ->      4700.0
4.7k     ->      4700.0
4.7R     ->      4.7
4R7      ->      4.7
0.47     ->      0.47
0R47     ->      0.47
4700k    ->      4700000.0
dhaillant commented 1 year ago

Nice! What about: 470m (0.47) 470M (470000k) and 4M7

Electro707 commented 1 year ago
470m     ->      0.47000000000000003
470M     ->      470000000.0
4M7      ->      4700000.0
dhaillant commented 1 year ago

@Electro707 How could I test your version? Thanks :)

Electro707 commented 1 year ago

@dhaillant You can clone my fork of this project and checkout the fix_117 branch: https://github.com/Electro707/PcbDraw/tree/fix_117

Then you can run pcbdraw directly from the cloned folder by running python -m pcbdraw.ui with the current path being inside the repository.

dhaillant commented 1 year ago

Thank you very much @Electro707 ! However, I get this error when I try either plot or populate commands:

david@b101p02:~/tmp/test/PcbDraw$ python -m pcbdraw.ui populate ~/elec/simple\ vca/1.0/populate.txt ~/elec/simple\ vca/1.0/img/ wx._core.wxAssertionError: C++ assertion "false" failed at /build/kicad-YdG4kW/kicad-6.0.8-0/kicad/common/layer_id.cpp(180) in LayerName(): Unknown layer ID -1

The above exception was the direct cause of the following exception:

Traceback (most recent call last): File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main return _run_code(code, main_globals, None, File "/usr/lib/python3.8/runpy.py", line 87, in _run_code exec(code, run_globals) File "/home/david/tmp/test/PcbDraw/pcbdraw/ui.py", line 312, in run() File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1128, in call return self.main(args, kwargs) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1053, in main rv = self.invoke(ctx) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1659, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1395, in invoke return ctx.invoke(self.callback, ctx.params) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 754, in invoke return __callback(args, kwargs) File "/home/david/tmp/test/PcbDraw/pcbdraw/populate.py", line 300, in populate parsed_content = generate_images(parsed_content, board, prepare_params(header["params"]), File "/home/david/tmp/test/PcbDraw/pcbdraw/populate.py", line 193, in generate_images generate_image(boardfilename, x["side"], x["components"], File "/home/david/tmp/test/PcbDraw/pcbdraw/populate.py", line 212, in generate_image plot.main(args=plot_args) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1053, in main rv = self.invoke(ctx) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 1395, in invoke return ctx.invoke(self.callback, ctx.params) File "/home/david/.local/lib/python3.8/site-packages/click/core.py", line 754, in invoke return __callback(*args, **kwargs) File "/home/david/tmp/test/PcbDraw/pcbdraw/ui.py", line 174, in plot image = plotter.plot() File "/home/david/tmp/test/PcbDraw/pcbdraw/plot.py", line 1053, in plot plotter.render(self) File "/home/david/tmp/test/PcbDraw/pcbdraw/plot.py", line 674, in render self._plotter.execute_plot_plan(to_plot) File "/home/david/tmp/test/PcbDraw/pcbdraw/plot.py", line 1207, in execute_plot_plan pctl.OpenPlotfile(action.name, pcbnew.PLOT_FORMAT_SVG, action.name) File "/usr/lib/python3/dist-packages/pcbnew.py", line 7586, in OpenPlotfile return _pcbnew.PLOT_CONTROLLER_OpenPlotfile(self, aSuffix, aFormat, aSheetDesc) SystemError: returned a result with an error set

I'm not familiar with python, so I have no idea if I'm using it the right way... (python -m pcbdraw.ui doesn't generate this error)

yaqwsx commented 1 year ago

The reason for that is incompatibility introduced in KiCAD 6.0.8. The current PcbDraw upstream already addresses this, @Electro707's fork, did not.

Electro707 commented 1 year ago

@dhaillant Which branch are you on? If you are on master, that would make sense as I haven't updated it in a while.

dhaillant commented 1 year ago

@Electro707 I'm on fix_117 As @yaqwsx said, it was an incompatibility issue introduced in KiCad 6.0.8.