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

preserve dmr extended id when applying changes with dmr mode disabled #103

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

Conversation

narspt
Copy link
Contributor

@narspt narspt commented May 13, 2020

Currently DMR extended id is discarded every time we apply changes with DMR mode disabled but either YSF or P25 modes enabled (so that the main id field shows and is posted), it's annoying to always lose DMR extended id when switching modes :)

currently dmr extended id is discarded every time we apply changes with dmr mode disabled but either ysf or p25 modes enabled (so that the id field shows), it's annoying to always loose dmr extended id when switching modes :)
@AndyTaylorTweet
Copy link
Owner

The DMR ID is used for P25, it can also be used for YSF2DMR and YSF2P25 also.
Where the main ID is set, it should only ever be 7 digits long, and not extended.

@narspt
Copy link
Contributor Author

narspt commented May 13, 2020

Yes, I know it. My patch doesn't touch main id at all or changes any behavior, it is intended to just fix DMR id losing extended id when you Apply Changes with DMR mode disabled (and either YSF or P25 modes enabled).

If you see it with care, the line I changed (967) is actually executed when either DMR, YSF or P25 modes are enabled (the only cases where $_POST['dmrId'] is posted), and on that line without my small change $configmmdvm['DMR']['Id'] always loses extended id, but if DMR mode is enabled then ID is "recovered" just below at line 977, the problem happens if DMR mode is disabled and YSF/P25 enabled then it loses extended id at line 967 and doesn't recover at line 977, you see what I mean?
To reproduce it you just need to disable DMR mode and enable YSF/P25 then Apply Changes, then when you re-enable DMR and Apply Changes again the DMR extended id will be missing. After my patch it always preserves current extended id at line 967, but still updates main part of DMR id if main id is changed, then extended id is still updated at line 977 if it is posted else we keep existing one...

@AndyTaylorTweet
Copy link
Owner

Let me test that out - I am sure you are correct, but I want to see it before I pull this in...
thank you :)

@narspt
Copy link
Contributor Author

narspt commented May 13, 2020

Sure, thank you :)

@narspt
Copy link
Contributor Author

narspt commented Feb 4, 2021

@AndyTaylorTweet: sorry to persist but please don't forget to look at this, it's really annoying to always lose the extended id when switching modes from DMR to YSF and then back to DMR...

@AndyTaylorTweet
Copy link
Owner

I've tested this, but I dont see how this is supposed to work, the DMR ID field should only have a 7 digit (or maybe 6 digit if its a repeater) ID in it, the service should contain the SSID - what am I missing?

@narspt
Copy link
Contributor Author

narspt commented Feb 4, 2021

Read my 2nd comment with care where I tried to explain how the id is lost on the code and what is the idea of my fix to preserve it #103 (comment)

And you can test it easy, for eg. you have extended id set to 02, then you disable DMR and enable YSF, then switch it back by disabling YSF and re-enabling DMR (you will be applying settings with DMR option currently disabled, this is when the problem happens) and you will see extended id is lost and now set to None instead of 02 as it should.

@narspt
Copy link
Contributor Author

narspt commented Jun 7, 2021

dmrextid.zip
I'm probably not being able to explain the issue correctly, please look at the video and the image attached and I'm sure you will then understand what I mean. (I had to zip them as github doesn't accept the video file)

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