footprint add STEP only recognises *.stp suffix

Bug #1659027 reported by David Brown
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Undecided
Cirilo Bernardo

Bug Description

When adding 3D model files to footprint only files with suffix *.stp are recognised. Files with *.STEP suffix are added to the list but do not render / appear to be actually read.

Suggest *.STEP / *.step suffix added as valid options.

Revision history for this message
Nick Østergaard (nickoe) wrote :

What platform and version?

.step works for me on archlinux with:
Application: pcbnew
Version: (2017-01-25 revision 456adf4b2)-master, debug build
Libraries: wxWidgets 3.0.2
           libcurl/7.52.1 OpenSSL/1.0.2j zlib/1.2.8 libpsl/0.16.1 (+libicu/58.2) libssh2/1.8.0
Platform: Linux 4.8.13-1-ARCH x86_64, 64 bit, Little endian, wxGTK
- Build Info -
wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8)
Boost: 1.62.0
Curl: 7.52.1
KiCad - Compiler: GCC 6.2.1 with C++ ABI 1010
        Settings: USE_WX_GRAPHICS_CONTEXT=OFF
                  USE_WX_OVERLAY=OFF
                  KICAD_SCRIPTING=ON
                  KICAD_SCRIPTING_MODULES=OFF
                  KICAD_SCRIPTING_WXPYTHON=ON
                  KICAD_SCRIPTING_ACTION_MENU=OFF
                  BUILD_GITHUB_PLUGIN=ON
                  KICAD_USE_SCH_IO_MANAGER=ON
                  KICAD_USE_OCE=ON

tags: added: 3d-viewer oce
removed: 3d stp
Revision history for this message
David Brown (dbrown2k) wrote :

This is on win7 64bit, both latest stable and nightly from a couple of days ago.

I've made a couple of simple boxes to test with.

test.STEP - direct export from solidworks; does not show anything

test.stp - direct export (same dialogue) but changing the suffix; this works

test - Copy.STEP - copy of unmodified test.STEP with re-named suffix; this works

Its very odd as soon as the file name is changed; introduction or removal of spaces it works some times and others it does not.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

I can confirm this on windows 7 pro as well. Another interesting thing is that the .STP extension does not work either. I'm a bit surprise the case of the file extension made a difference. Only the .stp extension seems to work correctly on windows. I haven't tested it on Linux yet.

Application: kicad
Version: (2017-01-25 revision 976d9fc)-master, release build
Libraries: wxWidgets 3.0.2
           libcurl/7.51.0 OpenSSL/1.0.2j zlib/1.2.11 libssh2/1.8.0 nghttp2/1.18.0 librtmp/2.3
Platform: Windows 7 (build 7601, Service Pack 1), 64-bit edition, 64 bit, Little endian, wxMSW
- Build Info -
wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8)
Boost: 1.60.0
Curl: 7.51.0
KiCad - Compiler: GCC 6.3.0 with C++ ABI 1010
        Settings: USE_WX_GRAPHICS_CONTEXT=OFF
                  USE_WX_OVERLAY=OFF
                  KICAD_SCRIPTING=ON
                  KICAD_SCRIPTING_MODULES=ON
                  KICAD_SCRIPTING_WXPYTHON=ON
                  KICAD_SCRIPTING_ACTION_MENU=ON
                  BUILD_GITHUB_PLUGIN=ON
                  KICAD_USE_SCH_IO_MANAGER=OFF
                  KICAD_USE_OCE=ON

Changed in kicad:
status: New → Confirmed
Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

On Windows, does the "step" extension work by any chance? I'll look into this; this is bizarre. On Linux both the STEP and step extensions are registered since most filesystems used on Linux are case sensitive, but on Windows only 'step' is registered. It's just plain weird that the file can be selected but isn't being loaded.

Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

One more question: in the file browser on Windows, do both STEP and step files appear or is there some case sensitivity going on?

One possible failure point is in the plugin's fileType() function (in loadmodel.cpp). The file is opened via 'fopen()' using a UTF8 string and if gcc simply invokes the Windows fopen() command then UTF8 strings will fail if any non-ASCII characters are present. I can rewrite that code to either use 'wx/textfile.h' or else use 'richio'. I'd rather use the wx functions since this is a simple test routine and not a full-fledged parser.

Revision history for this message
David Brown (dbrown2k) wrote :

Generating test files with *.STEP and *.step the lowercase instance loads happily.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote : Re: [Bug 1659027] Re: footprint add STEP only recognises *.stp suffix

On 1/25/2017 6:12 PM, Cirilo Bernardo wrote:
> One more question: in the file browser on Windows, do both STEP and step
> files appear or is there some case sensitivity going on?

I can enter both upper and lower case file extensions in the windows
file browser and it is displayed correctly.

>
> One possible failure point is in the plugin's fileType() function (in
> loadmodel.cpp). The file is opened via 'fopen()' using a UTF8 string and
> if gcc simply invokes the Windows fopen() command then UTF8 strings will
> fail if any non-ASCII characters are present. I can rewrite that code to
> either use 'wx/textfile.h' or else use 'richio'. I'd rather use the wx
> functions since this is a simple test routine and not a full-fledged
> parser.
>

I would prefer you use richio but I'm also fine with wxInputStream
rather than wxTextFile. Streams are more flexible. You never know when
you might want your 3D model to come from a source other than a file say
a string from the clipboard, compressed file stream, etc. If you use
wxFile, you will have to change your code yet again in the future if we
want to support something other than files. For now you can just create
a wxFileInputStream object and pass it to the parser.

Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

I'll use wxFileInputStream for now. The only issue I see with generic stream support is that OCE only wants a file for input and does not support any other schemes, so even if the file type test could read a stream from some other source, OCE would fail.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

I guess there is not much choice if OCE can only read files. It would
be useful if their parsers and lexers would use streams instead of files
but we have no control over that. It might be useful to add it to the
oce wishlist.

Does oce parse strings? If so, you could provide an intermediate
conversion from a stream to a string and pass that to oce.

On 1/26/2017 4:30 PM, Cirilo Bernardo wrote:
> I'll use wxFileInputStream for now. The only issue I see with generic
> stream support is that OCE only wants a file for input and does not
> support any other schemes, so even if the file type test could read a
> stream from some other source, OCE would fail.
>

Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

I think we need a clear idea of how to work with streams and how different stream data might be handled. In the past, networked virtual directories were popular for local nets but of course the data rates can be very high. If we plan to pull things off the internet I think we need to think about how to manage data usage; what to transfer and when, and how to handle situations where the remote data is no longer there. For many programs, streams must also be seekable. We can break so many things in a very bad way and I'm not currently convinced that the github retrieval of footprints works in a reasonable way. Features are nice, but if they introduce too many additional failure mechanisms we've got to put more thought into things.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

Yes we will have to carefully consider any stream, but stream
availability and I/O failures would be handled by the object derived
from the base stream object. Even file streams can have issues since
the file could just as easily be on a remote drive that is no longer
available or fails during the file read. Initially file and string
streams are adequate for what we are currently doing but using streams
or a stream type I/O prevent us from having to completely rewrite the
plugin I/O code when we want to add features like socket or archive
streams or some stream we haven't even dreamed up yet.

On 2/1/2017 4:12 PM, Cirilo Bernardo wrote:
> I think we need a clear idea of how to work with streams and how
> different stream data might be handled. In the past, networked virtual
> directories were popular for local nets but of course the data rates can
> be very high. If we plan to pull things off the internet I think we need
> to think about how to manage data usage; what to transfer and when, and
> how to handle situations where the remote data is no longer there. For
> many programs, streams must also be seekable. We can break so many
> things in a very bad way and I'm not currently convinced that the github
> retrieval of footprints works in a reasonable way. Features are nice,
> but if they introduce too many additional failure mechanisms we've got
> to put more thought into things.
>

Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

This patch should fix this bug and a (unreported) bug involving non-ASCII characters in filenames on Windows. Please test and let me know if all is OK.

Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

Sorry - the patch results in double-listing of files under Windows. The final solution is a little more complicated than I first imagined. I'll be back ...

Revision history for this message
Cirilo Bernardo (cirilo-bernardo) wrote :

OK, this patch had been tested and fixed this bug. Please test and comment.

Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

Fixed in revision 2b2b73ee4b72baf3c1f763ac2d2de3973e49ea48
https://git.launchpad.net/kicad/patch/?id=2b2b73ee4b72baf3c1f763ac2d2de3973e49ea48

Changed in kicad:
status: Confirmed → Fix Committed
assignee: nobody → Cirilo Bernardo (cirilo-bernardo)
Changed in kicad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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