clj / kom2

Interface KiCad database libraries with non-ODBC systems using magic
MIT License
6 stars 1 forks source link

Windows Support #2

Open SchrodingersGat opened 1 year ago

SchrodingersGat commented 1 year ago

Would love to see Windows support, and happy to provide time / code to implement.

@clj any pointers where to start? I have zero experience with Go or ODBC ;)

clj commented 1 year ago

I did put up a windows binary the other day after fixing a few things. (Check the current release). I forgot to ping in the InvenTree thread about that. I haven’t tested it (or any of the other binaries 😇). So if you are able to test it that would be fantastic. I don’t have a windows machine on hand to do it myself atm. But it ought to just work.

A PR or just dumping a draft here of windows install instructions would also be a big help. I guess this might mostly be around where to get an odbc manager.

SchrodingersGat commented 1 year ago

Alright @clj here's what I have tried sofar:

Download Driver

Download kom2.dll to local directory (e.g. c:\kicad\kom2.dll)

Install Custom ODBC Driver

(Run from a command prompt with administrator rights)

odbcconf INSTALLDRIVER "inventree|Driver=C:\kicad\kom2.dll|Setup=C:\kicad\kom2.dll|APILevel=2|ConnectFunctions=YYY|DriverODBCVer=0.35.0|FileUsage=0|SQLLevel=1"

(Instructions mostly taken from here)

Create DSN

(this is where is breaks ;))

image

image

SchrodingersGat commented 1 year ago

I'm not sure what these "setup functions" are supposed to be - any idea?

clj commented 1 year ago

@SchrodingersGat thanks a lot for giving this a spin. I do think I have an idea what the setup functions are, they are mentioned in the ODBC docs. I have skipped implementing those (and lots of other functions that are not used by KiCad) so that is probably what it is complaining about. I have downloaded a Windows dev image so I can pinpoint what functions are missing and have a go at implementing them.

Currently the automated build and release system (using GH actions) probably won't be able to do a Windows release, since I think implementing the setup functions will require linking against Windows APIs. I can't do that in the current CI build environment.

SchrodingersGat commented 1 year ago

@clj ping me when you have something for me to test - I'm very motivated to get this up and running :)

SchrodingersGat commented 1 year ago

@clj I have just tried latest release: https://github.com/clj/kom2/releases/tag/release-20230602

Unfortunately still hitting the same error.

Anything you can suggest for me to debug?

image

clj commented 1 year ago

Hey @SchrodingersGat thanks for testing again. I wanted to post an update since I haven’t actually managed to work on the windows installer component. Sorry.

I have though got the windows builds to work again (by changing the build infrastructure) and thereby gotten the ability to link against libraries in windows which was a prerequisite for this work.

In parallel I have set up a windows VM for development and testing. Which required me to buy some more RAM and debug some graphics issues with Windows 11 and Virtualbox. So this has taken much longer than I’d thought.

I’m now finally ready to try to work on this and I hope to have a bit of time over the rest of the weekend. Thankfully the SQLite odbc driver has some code I can use as an example, but I don’t think I’ll get it done by the end of the weekend.

I’ll post something here when I have put up stuff to test.

SchrodingersGat commented 1 year ago

@clj I look forward to it :)

SchrodingersGat commented 1 year ago

Hey @clj any updates on this? :)

clj commented 1 year ago

Hey @SchrodingersGat im really sorry but I haven’t had time to look at this in detail yet. I’m trying to ship a board (for which I’m very happily using all this stuff, including: https://github.com/clj/inventree-bom) and between that and life it’s not easy to find a few spare hours :). I’m hoping to get that out today though and then have some time to look at the windows stuff.

clj commented 1 year ago

@SchrodingersGat I have finally managed to make a bit of progress on this. I haven't tested it with KiCad yet, but I'm pretty sure there is still a step to register a DSN missing. But I can install and uninstall the driver and see it show up in the "ODBC Data Source Administrator". One snag is that the inst command needs to be run as administrator (e.g. using powershell started with "Run as Administrator"). To avoid having to instruct people to do that, it would probably be good to wrap this all up into a small installer which can then ask for the necessary permissions.

I'll ping here again when I have actually had time to test this with KiCad.

The latest build on my test windows related branch has the file included in the test-release-zips artifact.

SchrodingersGat commented 1 year ago

Nice work @clj I'll eagerly await your progress :)

clj commented 1 year ago

So I have determined that setting the DSN isn’t required. Which is nice because that would be a major pain to do (involving writing a small gui dialog box for entering the details). Instead the connection string in the .kicad_dbl file works fine.

Of course nothing is ever that easy. KiCad crashes at some point while interacting with the odbc driver. I haven’t got around to figuring out how to determine where. The windows builds have sentry reporting built in, but I don’t get a prompt about that, so I’m going to have to figure out how to attach a debugger or otherwise get hold of a stacktrace at the point here the crash happens.

clj commented 1 year ago

@SchrodingersGat I finally got the windows version working and made an installer to get everything set up correctly. You can grab a copy for testing from the releases page. I also updated the docs slightly to show how things should be configured for Windows (that could all do with some improvement)

I have downloaded and installed it and opened a project with a simple database config and that works for me. I haven't tested it more than that. If you do download it, let me know how you get on.


The two things that took me nearly a month to figure out and fix were:

The open source SQLite3 ODBC driver has been super helpful in figuring all this out.

SchrodingersGat commented 1 year ago

@clj thanks so much for putting the effort in here. I kind of have this running.

I can see the library in my symbol selection window, but no actual available parts.

Here's my .kicad_dbl file:

inventree.kicad_dbl.txt

And I have added Symbol and Footprint parameters to at least one parts:

image

But no available parts:

image

Am I missing a step here?

clj commented 1 year ago

@SchrodingersGat Thanks a lot for testing things! I'm a bit surprised that you're not seeing any components at all. I definitely see some when I connect to my own instance. If you have some time to enable logging and post the results that would be extremely useful. One source of logging is in the "ODBC Data Source Administrator" which has a "Tracing" tab where you can turn on tracing. This shows all calls being made from the application to the ODBC driver. I also have some sporadic and rudimentary logging in the driver itself, you can enable that by passing:

logfile=.../kom2.log;logformat=pretty;loglevel=debug

If you are able to get a trace and log file from both a non-crash and a crash that will hopefully help shed some light on things. If you could truncate the logs before starting KiCad each time that would be useful.

One other things that is different about Windows vs unixOBDC is that in Windows the ODBC manager doesn't do anything regarding concurrent calls to the driver, whereas unixODBC serialises calls (as far as I understand things). I didn't think this would be an issue as I would not imagine KiCad actually making concurrent calls, but perhaps this is something I have to look at some more.

SchrodingersGat commented 1 year ago

SQL.LOG.txt

Does this log file help at all?

clj commented 1 year ago

SQL.LOG.txt

Does this log file help at all?

Was that from a crash? The SQLDriverConnectW function call returned an error. There are a number of ways in which it can return an error, but I can't see which one, possibly because I haven't implemented the function that fetches the specific error code and message correctly. I haven't added any logging to those functions, so I might try to do that and then push a new release. Hopefully tonight, but I can't guarantee anything.

I failed to mention that the other log options:

logfile=.../kom2.log;logformat=pretty;loglevel=debug

should go in the connection string. But it won't log anything at the moment anyway in the SQLDriverConnectW function until I add some more logging.

clj commented 1 year ago

@SchrodingersGat I pushed another release which mostly has logging added to the connect functions so that it might be easier to see why SQLDriverConnectW returns an error.

When you have a minute, if you could try adding something like:

logfile=C:/temp/kom2.log;logformat=pretty;loglevel=debug

to the connection string and attach that log, I'm hoping that will help...

SchrodingersGat commented 1 year ago

@clj please find logs attached. I have tried against the demo server and a local server, not much luck so far:

image

You can see that it is hitting against the server, though:

image

I've attached my .kicad_dbl configuration file also. Note that I have tried ['id', 'pk', 'IPN'] values for the "key" parameter. Same result.

I am assuming that on the database side I need to set the symbol and footprint via parameters? Is the below screenshot correct?

Finally, do I need to set the part category ID? Or is name lookup sufficient?

Attachments

image kom2.log.txt inventree.kicad_dbl.txt

clj commented 1 year ago

@SchrodingersGat Thanks very much for your testing and patience!

I have been so focused on trying to diagnose the random crashes you reported, that I failed to notice that the config in the README.md isn't properly set up for the demo server. Your question about category ID and the attached file finally made me notice that. I have update the README file and I just tested it.

As to your questions:

I am assuming that on the database side I need to set the symbol and footprint via parameters? Is the below screenshot correct?

Yes, that looks correct.

Finally, do I need to set the part category ID? Or is name lookup sufficient?

Name lookup is sufficient. The problem was that I had set the "table" in the example config to "Resistors". Now this category does exist, but is actually called "Electronics/Passives/Resistors". If you put that in the config then things start working (🤞). The way that works is that all components in a category and its subcategories are displayed, so if you put "Electronics/Passives" you would get all passives, including resistors.

I think perhaps the documentation needs improving 🤔

Finally, are you still seeing random crashes? I haven't had any crashes for a while. I have tested fairly extensively on macOS, but not very much on Windows.

SchrodingersGat commented 1 year ago

I have updated my configuration file thusly:

image

To match the server:

image

And I see that it is polling the correct part category:

image

But still no parts showing up in the library selector.

I note in the image above that there are no API calls for the parameter data? For example the API is being queried with /api/part/?category=5 but to get the parameter data it should be /api/part/?category=5&paramters=true.

Or, is the parameter fetching intended to be performed as a separate API request?

SchrodingersGat commented 1 year ago

Finally, are you still seeing random crashes? I haven't had any crashes for a while. I have tested fairly extensively on macOS, but not very much on Windows.

Sometimes, but KiCad crash reporting sucks so I cannot track down why. One definite fault was that I initially had it pointing to localhost as https but it was hosted as http. This caused a silent crash which I only found by looking into the kom logs.

SchrodingersGat commented 1 year ago

Ok, some more progress. I had the "key" value set improperly in the kicad_dbl file. Set this to "pk" and now we get some parts being loaded!

image

Good so far. However the "name" value is not displaying correctly:

image

Individual part lookup seems to be working ish (but the "symbol" data are not coming through, it seems)

image

Notes

Part Parameter Query

You can combine these two queries into a single query:

image

e.g. /api/part/7/?parameters=true will include parameter data in the returned response

Interesting Crash

I saw an interesting crash the first time I loaded the data. The following request was made to the API, and then KiCad crashed:

image

  1. Perhaps there is some integer bug in your code, requesting the wrong ID?
  2. Does your interface handle a 400 response (or any non-200 response) from the server?
SchrodingersGat commented 1 year ago

Update again: I worked out the issue with the symbol not loading properly:

image

This is starting to look legit :)

clj commented 1 year ago

@SchrodingersGat its great to see some progress!

I'll answer your questions and notes in no particular order below:

One definite fault was that I initially had it pointing to localhost as https but it was hosted as http. This caused a silent crash which I only found by looking into the kom logs. Does your interface handle a 400 response (or any non-200 response) from the server?

I have not been focusing on dealing with errors, so it is very possible that a non-200 response will just make the driver very unhappy. I'll have a look at making it more resilient against non-200 responses.

Good so far. However the "name" value is not displaying correctly:

Yeah, KiCad uses the library "name" and the value of the "key" field separated by a slash in the symbol browser. It isn't great, which is why IPNs at least seem a bit better than using the pk from the DB, which is extremely opaque. At least the description is there on the right 🤷 There isn't really anything the plugin can do here to help as far as I can see.

You can combine these two queries into a single query:

I thought I'd already combined those, but perhaps not everywhere. I'll have a look!

Perhaps there is some integer bug in your code, requesting the wrong ID?

That ID does look very suspect! I wonder what happened there. I'll have a go at tracking that bug down.

Thanks a lot for your time testing this and your comments!

SchrodingersGat commented 1 year ago

I'll have a look at making it more resilient against non-200 responses.

While annoying, this will be a very important consideration. Silent crashes will make the process very frustrating especially for non-technical users.

Yeah, KiCad uses the library "name" and the value of the "key" field separated by a slash in the symbol browser.

Ah, ok I see. So if for example my parts don't have IPN values, I can use "name" attribute here instead?

SchrodingersGat commented 1 year ago

Very happy with how this is coming along. Once we have some of these kinks ironed out, are you happy for us to feature this on our blog? I'd also like to cross post over to the kicad forums :)

clj commented 1 year ago

While annoying, this will be a very important consideration.

Yeah totally agree. It's more a question of finding the time to be able to prioritise that. But at least now that things are broadly working in Windows, I can start looking at those things. Those errors will likely be present on all platforms, so definitely important to fix.

Once we have some of these kinks ironed out, are you happy for us to feature this on our blog? I'd also like to cross post over to the kicad forums :)

Definitely! Thanks a lot for helping with getting things ironed out. I primarily didn't post on the KiCad forums yet because it seemed sensible to limit the audience while ironing things out.

Ah, ok I see. So if for example my parts don't have IPN values, I can use "name" attribute here instead?

Not unless the name is guaranteed to be unique. Furthermore, the driver currently only supports pk (definitely unique) and IPN (probably unique, more probably if the right setting in InvenTree is enabled) as the "key".

What exactly does "Allow Duplicate IPN" do? does it add a constraint in the DB? and what if there are already duplicates in the DB when that is turned on?

SchrodingersGat commented 1 year ago

What exactly does "Allow Duplicate IPN" do? does it add a constraint in the DB? and what if there are already duplicates in the DB when that is turned on?

It allows parts to be created with duplicate IPN values. It is at the ORM level, not the DB level. Any parts with duplicate IPN values will retain them if the setting is turned on.

matmair commented 1 year ago

Maybe a coordinated blog release would make sense? Do you have a timeframe for readiness in mind @clj? I have a plugin to get install instructions and a a config file in place.

clj commented 1 year ago

@matmair I could see a coordinated blog release making sense. I don't have a timeframe for readiness sadly, I'm just working on this when I can find a spare few hours. :)

@SchrodingersGat we might need to have a chat about what we can use as unique identifiers for a part. It might make sense to switch that conversation to another issue than this one though: https://github.com/clj/kom2/issues/9

You can combine these two queries into a single query:

I thought I'd already combined those, but perhaps not everywhere. I'll have a look!

I just noticed why I haven't done that yet: https://github.com/clj/kom2/issues/6

matmair commented 1 year ago

Feel free to tag me here once you are ready @clj

SchrodingersGat commented 1 year ago

@clj is it possible to allow lookup of the "category" by PK value? i.e. instead of Electronics/Passives/Resistors specify 5