big refactoring, first step in merging VNC and CLI versions of the code

mrosseel commented 1 year ago

NOTE: VNC GUI version is not working currently

What was done:

refactoring of all handpad interactions into its own class
if the '-fh' or fake handpad option is passed, the handpad output is shown in the console and can be navigated with the keyboard
created EFinder class containing all relevant methods
added argparse CLI interface to eFinder.py
extracted all GUI related methods to gui.py, still to be investigated how to further merge this

mrosseel commented 1 year ago

@WimDeMeester just added the PR for the refactoring. To be clear, this shouldn't be merged.

It would however help if you could test it, but not under a clear sky because there are surely plenty of bugs. I've tested as far as I could but of course goto's and real nexus interactions are impossible to test.

Next task is integrating the GUI again, but I feel most of the hairy parts are done.

WimDeMeester commented 1 year ago

It would however help if you could test it, but not under a clear sky because there are surely plenty of bugs.

I'll try to do this tomorrow evening!

mrosseel commented 1 year ago

It would however help if you could test it, but not under a clear sky because there are surely plenty of bugs.

I'll try to do this tomorrow evening!

great, ui is also kinda working (add the -g flag) but many missing pieces there. The non-ui version should be better to test. If you don't want to use the handpad you can use the arrow keys, enter and space for navigating the handpad, whose current state is shown in the console.

WimDeMeester commented 1 year ago

Tested it very quickly in a Virtual Machine. The poetry update worked fine, and I can start up eFinder.py.

WimDeMeester commented 1 year ago

Tested and some problems to start up 😛 This is the log: errors.log

mrosseel commented 1 year ago

seems like the nexus couldn't be found on port /dev/ttyS0 - but it could also be triggered by several incorrect logging statements in nexus.py. That's one of the files I can't test locally due to no nexus :)

Could you try again?

WimDeMeester commented 1 year ago

Progress! The new log: errors.log

mrosseel commented 1 year ago

Progress! The new log: errors.log

wgs84 should be fixed now, along with another bug that was lurking in the nexus code

WimDeMeester commented 1 year ago

Testing will be for tomorrow, as we are leaving to my parents for the Christmas celebrations. Merry Christmas to all of you, @AstroKeith and @mrosseel

WimDeMeester commented 1 year ago

New test, new error log: errors.log

WimDeMeester commented 1 year ago

Tested it again, no errors anymore.
I first started eFinder.py (so without the GUI). Everything started correctly, the camera and the Nexus were found, but the handpad does not work... Second test was to start the GUI version (using the -g option). Starts fine, but also here, the handpad does not work. I tried to take a fake image (of M 31 and of polaris), but I got a white image...

mrosseel commented 1 year ago

@WimDeMeester did some more work on the Gui, also extracted connection type from Nexus in its own classes, and made Nexus inherit from NexusInterface so the debug class can be swapped without python linter warnings.

About the handpad: I don't have one, and can't see anything wrong in the code. Could you run with the -x flag and see if there are errors? If not, check that the port is correct for the handpad, that it still works in old code etc. Might be something stupid but can't recreate. It's indeed correct that the arrow key navigation of the handpad only works with the -fh flag, because if you have a real one this should work :)

In short: I'm progressing, if you could do some more handpad testing and we fix that, we can move on to the correct functioning of gui-less and gui operation

mrosseel commented 1 year ago

update: found the handpad misunderstanding, there's indeed a conflict between regular handpad and handpad usage in UI, will think about how to solve it. GUI is progressing with capture, solve, align, anotate and display mostly working, todo is the measure operations and goto via align, goto via move. Most valuable would be non-gui testing, since most of the code is the same anyway.

WimDeMeester commented 1 year ago

I don't have a lot of time now, but I did some tests.
I tried the handpad version, but I still can not use the handpad at all... No reaction when I use the button. I'll try to check this this evening. I also tried the gui version. I selected polaris from the option. I expect to get the polaris picture shown, but I get a new picture taken from the camera. This should not be the case...

mrosseel commented 1 year ago

Test image was fixed, only worked with fake camera. Known issues:

exp range and gain range not getting loaded/shown correctly
Goto not working
handpad not working in headless mode nor gui mode

WimDeMeester commented 1 year ago

Very quick test. Using the example images now works without problems. When I show the image and I then select the 'Use current value' for the Polaris offset, there is a small GUI glitch:

I tried to measure the offset from the polaris test image and I get the following error:

WimDeMeester commented 1 year ago

I have fixed the problem with the handpad. We were first comparing strings with Enum values. I corrected that, so everything seems to work. Some things I found while playing with the handpad:

In the previous version, when you are in the start menu, you can move to the right to see the Nexus location and the other menus. Now, I first have to move to the left, before I can see the Nexus location (and then to the right for the rest). See https://github.com/WimDeMeester/eFinder#in-the-handpad-version
When trying to set the exposure (by moving down, and then right, then up to move the value), I get the following error message:

I can have a look at these things, but it will be for later this week.

I saw I messed up the fake handpad now... I need to think how to find a better solution...

WimDeMeester commented 1 year ago

Fixed the problem with the fake and real handpad. I reverted my changes and made sure that the fake handpad uses the same code as before. For the real handpad, I now send the DisplayButtons enum instead of a string.

WimDeMeester commented 1 year ago

@mrosseel , I will let you fix the problem with the up_down_inc() and flip(). I think we are missing an attribute parameter, but I don't know what this should be...

mrosseel commented 1 year ago

@mrosseel , I will let you fix the problem with the up_down_inc() and flip(). I think we are missing an attribute parameter, but I don't know what this should be...

I have already fixed the attribute param error locally. Thx for the handpad fix - still one issue preventing me from checking in, should be done tomorrow

WimDeMeester commented 1 year ago

I tried the new version. I first updated the poetry packages. I get the following error when using a fake Nexus (but the real camera and the real handpad), starting up the GUI version:

When I start the non-GUI version (still with a fake nexus), and I go to the settings pages, and then to exposure, I get the following error:

Last test I did was with the non-GUI version, with fake nexus and fake handpad. I get the following error when I push 'Down', 'Right', 'Up':

mrosseel commented 1 year ago

Sorry should have notified you earlier, it works locally but haven't checked in yet

On Sun, 8 Jan 2023, 20:46 Wim De Meester, @.***> wrote:

I tried the new version. I first updated the poetry packages. I get the following error when using a fake Nexus (but the real camera and the real handpad), starting up the GUI version: [image: image] https://user-images.githubusercontent.com/7261529/211215617-564434bb-1f9f-4bff-8121-76ce7937aca9.png

When I start the non-GUI version (still with a fake nexus), and I go to the settings pages, and then to exposure, I get the following error: [image: image] https://user-images.githubusercontent.com/7261529/211215666-f3cc593f-dd6a-41d5-b9ef-b81c5913b346.png

Last test I did was with the non-GUI version, with fake nexus and fake handpad. I get the following error when I push 'Down', 'Right', 'Up': [image: image] https://user-images.githubusercontent.com/7261529/211215736-0aba999b-f7e2-4f9c-ba2c-54eb8306612a.png

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1374912547, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXUSQ75KXFFSX6NEDH3WRMKRDANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

Sorry for the misunderstanding

WimDeMeester commented 1 year ago

Hi @mrosseel Did you had some time to work more on this? Your last messages says that you have something working locally, but that you didn't check it in yet...

mrosseel commented 1 year ago

Hi,

Lots of other work + blocked on the config file which still had some issues (working but not good enough), should have a bit more time next week

On Thu, 26 Jan 2023, 17:18 Wim De Meester, @.***> wrote:

Hi @mrosseel https://github.com/mrosseel Did you had some time to work more on this? Your last messages says that you have something working locally, but that you didn't check it in yet...

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1405255585, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXRKTLPOR4NYC6DJNVLWUKPVXANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

Hi @mrosseel,

Any status update on this? If you don't have the time at this moment, can you check in the latest version of the source code and write a list with things that still need to be done? I'll have a look at it then.

mrosseel commented 1 year ago

Hi,

Back from travel and holidays, should be able to do something wed/thu, will check-in intermediate code if it makes sense.

Mike

On Mon, 27 Feb 2023 at 13:52, Wim De Meester @.***> wrote:

Hi @mrosseel https://github.com/mrosseel,

Any status update on this? If you don't have the time at this moment, can you check in the latest version of the source code and write a list with things that still need to be done? I'll have a look at it then.

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1446272442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXTA7M5BEQBNDCZJDGLWZSPPNANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

mrosseel commented 1 year ago

some progress, still thinking about config stuff, maybe yaml/json would be better/more standard. Or reverting most of the changes... Will change the config values back to what they were and check in soon.

Mike

On Tue, 28 Feb 2023 at 20:47, Mike Rosseel @.***> wrote:

Hi,

Back from travel and holidays, should be able to do something wed/thu, will check-in intermediate code if it makes sense.

Mike

On Mon, 27 Feb 2023 at 13:52, Wim De Meester @.***> wrote:

Hi @mrosseel https://github.com/mrosseel,

Any status update on this? If you don't have the time at this moment, can you check in the latest version of the source code and write a list with things that still need to be done? I'll have a look at it then.

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1446272442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXTA7M5BEQBNDCZJDGLWZSPPNANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

Thanks, @mrosseel ! I think yaml would be a good option for the config files.

mrosseel commented 1 year ago

yaml configuration was added and I committed the code. Need to do more testing to make sure it works correctly, but you can already take a look if you want.

On Mon, 13 Mar 2023 at 09:07, Wim De Meester @.***> wrote:

Thanks, @mrosseel ! I think yaml would be a good option for the config files.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

Thanks, @mrosseel ! I will have a look this evening. What is still missing (besides the testing)?

mrosseel commented 1 year ago

what I know now:

config might be still broken in some places
thread complaint about the gui loop, should be fixed but gui seems to work
making sure all gui stuff is extracted and deleting the VNC class

On Mon, 13 Mar 2023 at 15:49, Wim De Meester @.***> wrote:

Thanks, @mrosseel https://github.com/mrosseel ! I will have a look this evening. What is still missing (besides the testing)?

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1466287606, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXRBPISJZGL2G25DOJDW34XV3ANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

Some first tests:

I only used the handpad. When going down and then twice to the right, I get the following error:
When using the handpad, I can take a picture with the ASI camera without problems. The connection to the Nexus also works without problems.
When using the GUI, and if I select M 31 as example image, I can do the platesolve, but the results are not shown in the GUI. I only see the time it took to do the platesolving.
When using the GUI, I don't see my eyepieces.
When using the GUI and clicking the Polaris image, and then pressing the 'Measure' button, I get the following error:

Do I take a look at the problems or do you prefer to have a look?

mrosseel commented 1 year ago

I'll fix some more config stuff and see if I can recreate this. Feel free to test again tonight and fix some things, but don't forget to update.

Mike

On Mon, 13 Mar 2023 at 20:57, Wim De Meester @.***> wrote:

Some first tests:

I only used the handpad. When going down and then twice to the right, I get the following error:

[image: image] https://user-images.githubusercontent.com/7261529/224816635-f326bccf-e693-4f43-a4a7-ecb9b4939e49.png

When using the handpad, I can take a picture with the ASI camera without problems. The connection to the Nexus also works without problems. - When using the GUI, and if I select M 31 as example image, I can do the platesolve, but the results are not shown in the GUI. I only see the time it took to do the platesolving. - When using the GUI, I don't see my eyepieces. - When using the GUI and clicking the Polaris image, and then pressing the 'Measure' button, I get the following error:

[image: image] https://user-images.githubusercontent.com/7261529/224818064-d4a07706-6f55-451b-99a9-58dea5de5d31.png

Do I take a look at the problems or do you prefer to have a look?

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1466869758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXRUYH3ERS44XBTADPTW3533DANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

mrosseel commented 1 year ago

pushed an update, config should work now (eyepieces), having some trouble with the sidereal update thread which is not working atm. Haven't retested your scenarios

Mike

On Tue, 14 Mar 2023 at 11:45, Mike Rosseel @.***> wrote:

I'll fix some more config stuff and see if I can recreate this. Feel free to test again tonight and fix some things, but don't forget to update.

Mike

On Mon, 13 Mar 2023 at 20:57, Wim De Meester @.***> wrote:

Some first tests:

I only used the handpad. When going down and then twice to the right, I get the following error:

[image: image] https://user-images.githubusercontent.com/7261529/224816635-f326bccf-e693-4f43-a4a7-ecb9b4939e49.png

When using the handpad, I can take a picture with the ASI camera without problems. The connection to the Nexus also works without problems.

When using the GUI, and if I select M 31 as example image, I can do the platesolve, but the results are not shown in the GUI. I only see the time it took to do the platesolving. - When using the GUI, I don't see my eyepieces. - When using the GUI and clicking the Polaris image, and then pressing the 'Measure' button, I get the following error:

[image: image] https://user-images.githubusercontent.com/7261529/224818064-d4a07706-6f55-451b-99a9-58dea5de5d31.png

Do I take a look at the problems or do you prefer to have a look?

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1466869758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXRUYH3ERS44XBTADPTW3533DANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

Just tested the latest version. The eyepieces are now indeed visible and can be used. I think that the other problems are still in the software.
I found another (cosmetic) problem. When starting up the GUI, the exposure and gain are a bit weird.

I'll start to investigate the problems one by one...

WimDeMeester commented 1 year ago

Things to fix (@mrosseel, please add other problems you know of):

[x] Cosmetic problem with the exposure and gain settings.
[x] When using the handpad, going down and once to the right result in an error.
- [ ] Changing parameters using the handpad does not work (Down, right, up).
[x] When using the GUI, the results of the platesolve are not shown in the GUI.
[x] Error when using the GUI, selecting the Polaris image, and then pressing the 'Measure' button.
- [x] The image is not shown when pressing the 'Measure' button.
- [x] When selecting the polaris option in the GUI, the image is not used to calculate the offset.
[X] Small cosmetic when pressing the 'Used Saved' option for the offset. See https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1468764272

WimDeMeester commented 1 year ago

Now the coordinates of the solved field are displayed.

The delta x,y are not correct yet.

WimDeMeester commented 1 year ago

Found another small problem in the GUI:

When clicking on the 'Used Saved' option for the offset, a part of the image is black.

WimDeMeester commented 1 year ago

Found another small problem in the GUI:

When clicking on the 'Used Saved' option for the offset, a part of the image is black.

Solved:

mrosseel commented 1 year ago

@WimDeMeester got sidetracked by a GUI threading issue which was due to the fact I was running the UI through 'ssh -Y', and it doesn't happen on the pi itself 🙄

cannot reproduce the handpad bug
added a '.editorconfig' file because our code formatting differs and that makes it hard when merging. Visual studio code has a plugin for this, neovim >0.9 does it automatically. Contents of the file is just defaults, feel free to change.

I'm checking the 'measure' bug now

WimDeMeester commented 1 year ago

@mrosseel I tested the measure bug, but it is still not working for me... I have the impression that when using the real camera, that no image is used, even when the polaris image is selected... When I use the fake camera, I get the needed information. Anyway, as far as I remember, the image should be shown when clicking the measure button. This is not the case at the moment.

mrosseel commented 1 year ago

@mrosseel I tested the measure bug, but it is still not working for me... I have

I fixed a crash, what you are describing is a new bug related to the measure button. Will retest, can you also check the handpad because I can't make it crash (I'm using the fake handpad of course). The exposure and gain radiobuttons have resisted my attempts to fix them up till now. Will have another look tomorrow.

WimDeMeester commented 1 year ago

For the handpad bug, I print out line1 and the type of line1. For the real handpad, I get

when I go down and once to the right.
For the fake handpad, the code in line 70 of Display.py is not executed.
I can check if line1 is an int, and if that is the case, I can convert it to a str. I need to do the same for a float then. I don't know if this is the best solution...

I also cannot change the parameters. For example, when I try to change the Exposure, I get

This problem also occurs with the fake handpad. When I go down, right and up, I get the same error as in the screenshot above.

WimDeMeester commented 1 year ago

I think I fixed the problem with the crashing handpad in 0372283. However, the values are not changed at all... I don't see how I can fix this. Can you have a look, @mrosseel ?

mrosseel commented 1 year ago

You mean the values don't change on the UI, the config file, thebhandpad or all of the above?

On Sat, 18 Mar 2023, 21:40 Wim De Meester, @.***> wrote:

I think I fixed the problem with the crashing handpad in 0372283 https://github.com/WimDeMeester/eFinder/commit/0372283fe99c97bfb0a1a8051f83b632fa71e19d. However, the values are not changed at all... I don't see how I can fix this. Can you have a look, @mrosseel https://github.com/mrosseel ?

— Reply to this email directly, view it on GitHub https://github.com/WimDeMeester/eFinder/pull/47#issuecomment-1474987162, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFXXQXN6QICNLYAH4ISPLW4YMU3ANCNFSM6AAAAAATGBM5PA . You are receiving this because you were mentioned.Message ID: @.***>

WimDeMeester commented 1 year ago

In any case not on the handpad... I did not check the file

WimDeMeester commented 1 year ago

In any case not on the handpad... I did not check the file

The file and the GUI are also not adapted.

WimDeMeester commented 1 year ago

The problem with the handpad is that the changes are not displayed on the handpad. If I change the value of the exposure, the same value is shown on the handpad. If I go back to the overview page, I can see the updated value.

WimDeMeester commented 1 year ago

Hi @mrosseel,

Is there still something you want to change? If not, I will accept this pull request and start working on the main branch to fix the last bugs.

WimDeMeester / eFinder

big refactoring, first step in merging VNC and CLI versions of the code #47