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

fix: add current drive detection xibosignage#238 #239

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

Stivius
Copy link
Collaborator

@Stivius Stivius commented Apr 13, 2021

The problem was that I'd always used harcoded /dev/sda as a main device where the player should be stored. This is wrong at least for 2 reasons:

  1. Player can be saved on a flash drive (which can be /dev/sdb or something else)
  2. /dev/sda is not always the name of the main drive

I'm using df /player/binary/location now to determine the storage device.

WARNING: potentially it could change display ID (hash will be changed) for some users - e.g. they had /dev/sda on the machine but were using /dev/sdb to store player binary. Because it should determine the location correctly now it will change the display's hash.

@Stivius Stivius requested a review from dasgarner April 13, 2021 14:50
@Stivius Stivius self-assigned this Apr 13, 2021
@dasgarner
Copy link
Member

WARNING: potentially it could change display ID (hash will be changed) for some users - e.g. they had /dev/sda on the machine but were using /dev/sdb to store player binary. Because it should determine the location correctly now it will change the display's hash.

If the display hash has already been calculated and stored in settings, then nothing should change that value unless settings is deleted. For example, the user might set a completely custom displayID instead of using the pre-generated one.

Please can you confirm that this PR will not change this behaviour, and will only generate a new hash for fresh installations, or for uninstall/reinstall where settings are removed?

@Stivius
Copy link
Collaborator Author

Stivius commented Apr 21, 2021

Yes, exactly. I should have mentioned that before. This behaviour is applied to new installation/settings update.

@dasgarner
Copy link
Member

Yes, exactly. I should have mentioned that before. This behaviour is applied to new installation/settings update.

Thank you - that makes sense to me then. I'll merge.

@dasgarner dasgarner merged commit 1f0e919 into xibosignage:master Apr 22, 2021
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.

2 participants