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

rmfakecloud-proxy:  Add status subcommand #513

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

matteodelabre
Copy link
Member

This PR addresses the points from @Eeems’ review of last PR: #448 (comment)

The new rmfakecloudctl status introduced by this PR reports whether rmfakecloud-proxy is enabled, currently active, and whether the upstream server has been set. If the upstream server is set, it also checks whether it is reachable and a valid rmfakecloud server. Here are some possible states that can be shown by the command:

Disabled (happens, for example, right after install):

rmfc-disabled

Enabled, but upstream not set:

rmfc-enabled-inactive

Enabled, but upstream is unreachable:

rmfc-enabled-unreachable

Enabled, but upstream does not seem to be an rmfakecloud server:

rmfc-enabled-invalid

Enabled and working:

rmfc-enabled-working

This PR also changes the disable command to not clean up the /opt/etc/rmfakecloud-proxy/config file. This fixes the issue where doing rmfakecloudctl set-upstream, followed by an interrupted rmfakecloudctl enable, would unset the upstream.

This subcommand reports whether rmfakecloud-proxy is enabled, currently
active, and whether the upstream server has been set. If the upstream
server is set, it also checks whether it is reachable and a valid
rmfakecloud server.
@matteodelabre matteodelabre added bug Something isn't working packages Add or improve packages of the repository labels Dec 16, 2021
@Eeems
Copy link
Member

Eeems commented Dec 16, 2021

Status command looks good, and it does indeed leave the configuration in place. Unfortunatly upon re-enable it shows me as no longer linked to the cloud. We might need to sort out how to properly clean up the xochitl configuration and reuse the pin.

I also noticed that when I tried to disable it got stuck waiting for xochtil to stop until after I entered my pin in Oxide's lockscreen. It might be good to have it detect that the application is in a stopped state, that or just detect that oxide is running and use rot to kill the application.

@matteodelabre
Copy link
Member Author

Thanks for testing!

Unfortunatly upon re-enable it shows me as no longer linked to the cloud. We might need to sort out how to properly clean up the xochitl configuration and reuse the pin.

That's a good point. To be on the safe side, I programmed the enable, disable, and set-upstream commands to disconnect the user from the cloud (by killing Xochitl and removing the usertoken and devicetoken fields from ~/.config/remarkable/xochitl.conf) before doing anything. This is to avoid situations where Xochitl would try to login to another server using credentials from the previous server.

We could implement something to save credentials for each server you connect to somewhere, and restore them when you switch between servers. But is there such a strong use-case for switching between multiple servers regularly enough that it’s worth implementing this? Also, the way it’s currently implemented, we force a full sync after each server switch, and assume that files on the device should overwrite those on the server. But for users who would switch between multiple servers I’m not sure what strategy would be best in that regard.

I also noticed that when I tried to disable it got stuck waiting for xochtil to stop until after I entered my pin in Oxide's lockscreen. It might be good to have it detect that the application is in a stopped state, that or just detect that oxide is running and use rot to kill the application.

It should have force-killed Xochitl after 5 seconds. Could you tell me how to reproduce this so that I can investigate? Thanks!

@Eeems
Copy link
Member

Eeems commented Dec 18, 2021

It probably did force kill after 5 seconds, it would still be nice if it could attempt to work with the launcher. See #515 (comment)

I think this is safe enough to merge for now. We can resolve the other issues later.

@Eeems
Copy link
Member

Eeems commented Dec 20, 2021

@matteodelabre poke

@matteodelabre
Copy link
Member Author

Seems good. Thanks for the review!

@matteodelabre matteodelabre merged commit 88f7642 into testing Dec 21, 2021
@matteodelabre matteodelabre deleted the package/rmfakecloud-proxy/fix-1 branch December 21, 2021 00:46
Eeems pushed a commit that referenced this pull request Jan 12, 2022
* Add `rmfakecloudctl status` subcommand

This subcommand reports whether rmfakecloud-proxy is enabled, currently
active, and whether the upstream server has been set. If the upstream
server is set, it also checks whether it is reachable and a valid
rmfakecloud server.

* Keep config on uninstall
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working packages Add or improve packages of the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants