levylabpitt / AFM-Lithography

BSD 3-Clause "New" or "Revised" License
7 stars 0 forks source link

Error in Setting Active Database #78

Closed ciozi137 closed 2 years ago

ciozi137 commented 2 years ago

image

This happens during Macro: Start Driver called by Macro: Initialize

Monkeymerlot commented 2 years ago

Replicated previously. Only happens for certain files, most notably when parsing circles (stand in for dots) sometimes. I have a file that causes this issue so I was going to debug it at some point but the initialize sequence is a huge mess right now because of the way I have asynchronous (for idle state of the program) and synchronous (for the start up routine, refresh routine) events set up. I'll try to find the file sometime today and put it here.

ciozi137 commented 2 years ago

This happens for me at startup with the Default files. ...Actually before any files are loaded I think

Monkeymerlot commented 2 years ago

You are correct. Just found the issue. UI: Update Subpanel calls Database: Set Active >> %d but we haven't parsed any files yet until we run Macro: Refresh, which is called after Database: Set Active >> %d in this case.

I hope you see what I mean by the start up routine is a mess. The main program is in between two architectures and doing them both poorly at the moment. I would love to have a meeting after my prelim to discuss architecture because I think it would with maintainability and readability.

Monkeymerlot commented 2 years ago

So that didn't fix anything...

Start up routine states: (emphasis added to error states)

Macro: Initialize
UI: Ready
Data: Initialize
Initialize Core Data
Configuration: Initialize
Configuration: Read
UI: Background Path
UI: Inkscape Path
Parser: Start
Database: Start
Driver: Init Manager
Events: Register
UI: Initialize
UI: Enable Run
UI: Front Panel State
Data: Set Root Path
Data: Run Edit VIs
UI: Update AFM Ring
Macro: Start Driver
Driver: Init Drivers
Driver: Set Active
Driver: Insert Into Subpanel
Macro: Refresh
Litho: Load SVG
UI: Inkscape Path
Parser: Parse SVG
Database: Refresh Sync
Litho: Load Background
UI: Background Path
Litho: Display Background
Litho: Update Information
Driver: Litho Data
Litho: Order Data
Driver: Litho Data
Litho: Refresh Render
UI: IMAQ Render
UI: Update Subpanel
Database: Set Active
--- Error Handler ---
Macro: Send Driver
Driver: Background Image
Driver: Litho Data
UI: Ready

UI: Update Subpanel
Database: Set Active
--- Error Handler ---
Litho: Order Data
Driver: Litho Data

Macro: Properties Updated
Idle
UI: Ready
Litho: Update Information
Driver: Litho Data
Litho: Order Data
Driver: Litho Data
Litho: Refresh Render
UI: IMAQ Render
Driver: Litho Data
UI: Ready
Macro: Properties Updated
Idle
UI: Ready
Litho: Update Information
Driver: Litho Data
Litho: Order Data
Driver: Litho Data
Litho: Refresh Render
UI: IMAQ Render
Driver: Litho Data
UI: Ready
Macro: Properties Updated
Idle
Macro: Properties Updated
Idle
UI: Ready
Litho: Update Information
Driver: Litho Data
Litho: Order Data
Driver: Litho Data
Litho: Refresh Render
UI: IMAQ Render
Driver: Litho Data
UI: Ready
UI: Ready
Litho: Update Information
Driver: Litho Data
Litho: Order Data
Driver: Litho Data
Litho: Refresh Render
UI: IMAQ Render
Driver: Litho Data
UI: Ready
UI: Update Subpanel
Database: Set Active
Litho: Order Data
Driver: Litho Data

UI: Update Subpanel
Database: Set Active
Litho: Order Data
Driver: Litho Data
Monkeymerlot commented 2 years ago

For now I am going to add an additional state for setting active values that takes an empty string an does nothing. That error was intended to prevent mistyped states (like c as in circle instead of d as in dot or s as in square instead of r for rectangle). It also served as a reminder that I needed to set the active value in the settings database when adding new objects to the program (such as dots). So I think with the intent considered the expectation (Ik, risky) that the settings database will use 0 index as the active value of that object type if not otherwise specified, then I think that this is OK. Long term though this whole entire thing is going to get switched out for a better architecture.

ciozi137 commented 2 years ago

Ok good I'm glad you found the error. I understand the hesitation to keep fixing bugs when the entire architecture will get updated in the future.

If it's ready I can build 10.0.1?

Monkeymerlot commented 2 years ago

I still need to implement the fix. Give me like 10 minutes and I should be ready.

And yeah the biggest issue is that some bugs are like fundamentally a bug, like they are bad programming, and some of them are just that we have asynchronous stuff mixed in with synchronous stuff and UI events and a lot of really fragile legacy code.

Another potential culprit (saving for later) image image This will then call database to set the active object.

Monkeymerlot commented 2 years ago

@ciozi137 fix is pushed. I guess I only needed 2 minutes...

ciozi137 commented 2 years ago

Awesome! This version starts without errors - those startup errors are very unnerving.

Monkeymerlot commented 2 years ago

OK so I did some clean up and investigation. So I saw one fairly big issue and then I also added functionality to check if an event has been read, so we don't keep updating properties when they have already been updated. This cut down on the start up sequence by a lot... Still not perfect though.

New Start up sequence:

UI: Update Subpanel
Database: Set Active
Macro: Start Driver
Driver: Init Drivers
Driver: Set Active
Driver: Insert Into Subpanel
Macro: Refresh
Litho: Load SVG
UI: Inkscape Path
Parser: Parse SVG
Database: Refresh Sync
Litho: Load Background
UI: Background Path
Litho: Display Background
Litho: Update Information
Driver: Litho Data
Litho: Order Data
Driver: Litho Data
Litho: Refresh Render
UI: IMAQ Render
Macro: Send Driver
Driver: Background Image
Driver: Litho Data
UI: Ready

UI: Update Subpanel
Database: Set Active
Litho: Order Data
Driver: Litho Data

Problems before:

Dot settings was not unbundled and wired to state machine data: image

Inside of refresh sync the for loop was only wired for 3 times, and since dots are sent last they were never recieved

image

Monkeymerlot commented 2 years ago

Merged into develop ad0ffb6