Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid disabling services from signalk errors, do not restart services #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

seandepagnier
Copy link
Contributor

These changes needed or pypilot is reset and disabled when it should not be.

@sailoog
Copy link
Collaborator

sailoog commented Feb 5, 2023

  • Why are you removing the code in charge of uninstalling openplotter-pypilot-read in pypilotPreUninstall? is this unintentional?

  • I am confused. You said in your last PR that openplotter-pypilot is broken without these changes but I can not reproduce your error and no one else has reported that. Are you sure you are using the last version? In openplotterPypilot.py we only disable pypilot and pypilot_boatimu until the Signal k connection has been established, after the connection is approved in SK, everything works perfectly. We do that to prevent pypilot to do its token request. In startup.py we only restart pypilot and pypilot_boatimu if the connection has just been approved because we overwrite the pypilot token with the openplotter-pypilot token. This restart will not happen again.

Maybe there is something unusual in your dev environment or you are not using latest versions. Let me know where you get the error and details about the scenario.

@Paradoxe120
Copy link

Paradoxe120 commented Feb 6, 2023 via email

@sailoog
Copy link
Collaborator

sailoog commented Feb 6, 2023

There is not solution for that. Pypilot uses the UART0 and the Moitessier Hat hack also uses the UART0. If you want to use both devices you should write to https://www.rooco.eu and ask for a driver for your kernel so that you can use SPI0 for the Moitessier instead of UART0. They have stopped distributing drivers but maybe if you insist they can do something.

@Paradoxe120
Copy link

Paradoxe120 commented Feb 6, 2023 via email

@seandepagnier
Copy link
Contributor Author

  • Why are you removing the code in charge of uninstalling openplotter-pypilot-read in pypilotPreUninstall? is this unintentional?

I thought we determined you can just run pypilot instead of this script and it is easier to maintain that way.

  • I am confused. You said in your last PR that openplotter-pypilot is broken without these changes but I can not reproduce your error and no one else has reported that. Are you sure you are using the last version? In openplotterPypilot.py we only disable pypilot and pypilot_boatimu until the Signal k connection has been established, after the connection is approved in SK,

This is the problem. pypilot should not require the user to configure signalk. I think people will not understand why pypilot is not working and not realize why.

They may choose not to use signalk and pypilot could work without it, but either way I am not sure what disabling pypilot until signalk is approved. I dont think it is the right thing to do.

@sailoog
Copy link
Collaborator

sailoog commented Feb 8, 2023

I thought we determined you can just run pypilot instead of this script and it is easier to maintain that way.

Nop, we determined to use temporally this script until you add Signal K communication to the "IMU only" mode. I will maintain this script, no problem. Remember that there are people that only use the IMU feature and it is not using the autopilot feature of pypilot. We can not just use the autopilot mode in these cases for these reasons:

  • We will waste resources running unnecessary scripts.
  • We should remove the "IMU only" mode from the GUI and the manuals because it would no longer make sense to have different modes.
  • When the autopilot mode is enabled pypilot starts fighting for the serial devices and kills any connection made in Signal k, opencpn or any other program.

I think the current script is the cleaner and less intrusive way of making pypilot work in any scenario.

This is the problem. pypilot should not require the user to configure signalk. I think people will not understand why pypilot is not working and not realize why.

They may choose not to use signalk and pypilot could work without it, but either way I am not sure what disabling pypilot until signalk is approved. I dont think it is the right thing to do.

There are messages everywhere in openplotter that warn users why pypilot is not working yet and what is the next step they have to make. This only happens the first time they run openplotter-pypilot, after the connection is made, it will never happen again unless the connection fails.

When pypilot is working in autopilot mode in non openplotter systems, it does the same and automatically makes connection requests to SK without warning the user and creating multiple unattended notifications in the SK server. When a connection made by pypilot fails, the user is not warned either. That is OK because that is not the pypilot's job but it is the openplotter's job to make all programs work together and inform the user about what is happening in the system at any time. The SK connection management system in openplotter was designed to do this job and this is why we override the pypilot SK connection in a clean and non-intrusive way and this is why we temporally disable pypilot until the connection is made by openplotter-pypilot. It is just a one click process. This is how all openplotter apps work and users are already familiar with it.

Signal K is the core of openplotter and all apps assume the server is present and running. If any user do not want to use Signal K I do not see any reason to keep using openplotter.

I understand that pypilot must take care of everything when working in standalone mode in tinypilot but in openplotter it must coexist with more applications and I think we have achieved this without you having to make any changes to the pypilot code. If you do not agree we could explore different ways like having one openplotter app just for pypilot in autopilot mode and another openplotter app to manage just the "IMU only" mode but it sounds like adding too much complexity if they can work together as they work now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants