Parameter id duplication loading fix #329
Conversation
…le which contained a parameter that had a duplicate id. fix mock device not working, and use debug compiler directive to enable it instead of needing to uncomment lines. fix garbage collection issue with writing to the debug log when using the mock device (and improve performance using non mock devices) slow down the mock device a bit because going as fast as possible isnt realistic and also causes problems while trying to debug.
|
Thanks for your contribution! I have been watching your comments, but not sure what to add as I have not been part of writing logger, and also do not use it myself. But I am glad that you are helping us out. I have ping'd @Tazzi3 to get get thoughts on the XPro driver change as that will be a wide reaching change. Can you please confirm that you have tested this and it is working for you? Also, have you tested PCMHammer with the XPro change and is that still working for you? Assuming so, on which platforms have you tested it? I general I think we will certainly accept this pull request, but I'd like to wait a little longer to see if someone who uses logger can build it and test it and report back also. Thanks! |
|
Ive tested everything in logger, but have not tested in pcmhammer. I will do that before this is accepted. TBH I was focused on testing the logger. I'm still figuring out how all of this fits together. I certainly welcome more testing. My testing uses an obdxpro vt and an 0411 swapped 96 sierra, with os 12212156. |
|
I've tested pcmhammer with my changes. It appears to function correctly. I dont seem to have the assembly kernel in my bin folder, so it errors out there, but recieving messages seems to have no error. I dont have time to test further right now, but I can certainly do that later. |
There was a problem hiding this comment.
I am happy to accept this code, though I am not a user of logger myself. @LegacyNsfw are you happy to merge this?
|
See comment: 5f47e14#r124815628 |
A little stream of consciousness here, but essentially I started out wanting a way to fix the issue where if you selected a pid that had the same id as another pid, and then saved and loaded, there was a good chance of haning that other pid selected instead.
This fix is not ideal, IMO. I've basically added name into the key of the parameter. I've made sure that old config files will still load (they will still exhibit that bug, one time, and then they will save with the name as well)
The ideal fix (IMO) would be to rip the load save code out, as well as rejig the parameter database and profile objects to be serializable. After that it would be a single line to load and save both things. Parameters would need to be keyed still (name only, probably), but that would eliminate the issue on load and save, and as well, would enable saving the profile as a list of names only. That is significantly more work, but would enable the use of WPF instead of winforms, and databinding instead of ui-thread calls to update things. Way less code that way.
I've also included several more performance related fixes:
Mock device was not working for several reasons, and (feel free to correct me) IMO just using debug directives is easier and less bug prone than commenting and uncommenting.
there was a garbage collection issue with the ui-thread code to update the debug log.
the mock device was running at full speed ahead, and that was also taxing my poor laptop.
the waitforserial code on the obdxpro device was also using a full processor, a simple reorganize and sleep(1) cut that by 90%, now my laptop can happily log for much longer.
I hope I'm not presuming too much here. Let me know if any of the assumptions that I've made are wrong. I would love to work on modernizing this UI, and adding more to it.
fixes #328