-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
DietPi-Config | [RPi] Display Options: Rework to add supported HDMI modes based on auto-detection #2946
Conversation
- Add HDMI auto detection - Add array to handle the detection - Add checks to write values to config based on user selection - fallback if no hdmi is detected
HDMI Autodetection
@@ -822,160 +822,441 @@ A long (or insufficiently manufactured) cable may required a higher boost settin | |||
|
|||
fi | |||
|
|||
# RPi | |||
elif (( $G_HW_MODEL < 10 )); then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little alignment issue: One tab too much
local current_value=$(grep -m1 '^[[:blank:]]*dtoverlay=vc4-' /DietPi/config.txt | sed 's/^[^=]*=//') #OpenGL check 1st | ||
|
||
#If framebuffer is not set by user then check if hdmi default is detected and being used | ||
if [[ "$framebuffer_y" == 0 ]] && [[ "$framebuffer_x" == 0 ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bash magic:
- left sided variables never need to be quoted, multiple conditionals can be merged into one statement:
[[ $framebuffer_y == 0 && $framebuffer_x == 0 ]]
or as arithmetic check
(( $framebuffer_y == 0 && $framebuffer_x == 0 ))
although the arithmetic quecks print errors if values are no integers, thus for such simple checks I prefer string checks in most cases, especially here since for sure we want to use tvservice as fallback if for some reason the estimated framebuffer sizes were invalid values, e.g. broken config.txt file or such.
#get Y | ||
tvservice_s_framebuffer_y=${value_of_tvservice_s%@*} # retain the part before last character @ | ||
tvservice_s_framebuffer_y=${tvservice_s_framebuffer_y##*x} # retain the part after the last character x | ||
tvservice_s_framebuffer_y=${tvservice_s_framebuffer_y::-1} # Remove the last character of blank space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful bash string manipulation, muuuuch faster than any sed/awk/cut/... 👍
|
||
# - Check for headless | ||
grep -q '^[[:blank:]]*hdmi_ignore_hotplug=1' /DietPi/config.txt && grep -q '^[[:blank:]]*hdmi_ignore_composite=1' /DietPi/config.txt && current_value='Headless' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The downsite greps need to be replaced with the upside greps again. This was a recent change in dev code since the missing ^
in downside greps lead to wrong results if those two settings were commented. Also -i
not required since config.txt is case sensitive.
@meeki007 |
Thanks for all your help and understanding with the code. Taking the time to point out the mistakes good but also explaing why ' -i ' , grep carrot, bash left side vars ; all = excellent for my re learning of bash :) Makes me want to keep digging. I am far from done with this. I fly back home friday. Should have pi 4 next monday. Then the real fun starts :) |
|
||
) | ||
if [ "$DMT_detected" != '0' ]] || [[ "$CEA_detected" != '0' ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One missing bracket and as above:
[[ $DMT_detected != '0' || $CEA_detected != '0' ]]
or here shorter arithmetic check is possible to not list entries in case of invalid value:
(( $DMT_detected || $CEA_detected ))
General idea about applying the values: |
pi model 4 just showed up. Waiting on HDMI micro cables now. |
@meeki007 |
I like your idea of
My extra cheep Chinese 7in hdmi display for a car dash install will not auto detect but my LG in my room detects fine. Getting another hdmi screen tomorrow (not the same model i have) so i can see them both connected at the same time and decided how to design the array. Still pecking. Got a real life job so weekends are when I'll produce the most :) |
I would go with e.g. Ref: https://www.raspberrypi.org/documentation/configuration/config-txt/video.md |
Update: I've been tracking down the issue of screen selection and blank screen at boot of fresh dietpi.img to sd card. Issue If both screens are plugged in it will pick the one that posts first. Not based off of HDMI-0 or HDMI-1 if i swap the cables and put in a fresh sdcard with new image it still picks the faster to post screen if they are both on before I power up the PI 4. Now testing if it picks the screen that is on no mater what port the cable is plugged into if only one screen is on. Sorry but still teething this new system. |
Ok ignore above post. After 2 hrs of fumbling around i accidentally swapped the cables between devices and it started picking the other monitor. Turns out one of my brand new Philips micro-hdmi to hdmi cable was bad right out the box!!!! MOVING ON. thank you philips for the wasted time down that rabbit hole |
@meeki007 EDIT: Ah I was able to rebase, so everything is fine 🙂. |
Your going to want to hold off on that Push Request. I've completely re done the menu system for the PI section. Its close. Just working out a bug with 2 arrays that have to work in pair under the same loop. I'll get you Screen shots of what im up to tomorrow night. David |
@MichaIng
|
@meeki007
Why not turning it into a
More cleaned up would be probably the following:
|
I like your ideas above better. Should be easyer to do now that i have a better grasp of the menu system. Going tubing down a river with beer tomorrow so next code block Saturday |
@MichaIng Old Code optimizations
when it can be put in an array to be called from
Optimizing array counts and such. way faster to use: Other optimizations as well. Got tired of writing menu code that is not as fast a possible :) |
@meeki007 Okay nope one grep scrape is slightly faster:
|
Well guess it comes down to how many greps vs iterating through an array. I've also found its faster to
and store a value to variable than to.....
|
also i'm dealing with possible user input failures What if someone edits the config like this
well we don't want it seeing both or overiding the hot plug enabled. so I have to put checks in for that kind of user input issues |
@meeki007
can be used to apply a setting that way, that it automatically either changes non-commented settings, or turns commented entries into uncommented settings, if present, otherwise adjusts adds the setting to end of file. That way it prevents creating doubled entries, e.g. commented + non-commented. |
I'm thinking about a user editing the file directly. G_CONFIG_INJECT = ya did all the hard work for me !!! I did not understand its full logic until now. Thankyou!!! |
however this is working nicely
and i can keep adding checks to it as needed so it only runs through the array one time |
I got ya now!!! way less verbage
|
I can't murge the HDMI and SDTV SDTV has multiple modes and resolutions as well I'd like to get the OK on this menu so i can move on to the sub menu's below. What do you think? and if you enable headless this happens now :) This is the code in case i get hit by a bus
|
@meeki007 I have again another, perhaps slightly cleaner menu structure idea 😅:
If HDMI or Auto is chosen, the HDMI related menu entries appear, HDMI resolution, HDMI boost, HDMI rotation (not possible on SDTV I think?), 4k support (although 4k could be simply enabled automatically by adding this as fixed resolution on RPi4?) If SDTV or Auto is selected above, then the SDTV related menu entries appear, resolution (aspect ration + mode actually, if I see correctly?). And if any other than Headless is selected, the other video related settings appear as well, such as overscan and VC4 driver. Ah "Output" should as well allow to select "LCD" which then shows the LCD/touchscreen selection menu which already exist. Then only one "LCD model" entry and in case resolution (toggles framebuffer only then) + rotation + overscan additionally for custom/unknown LCD models. Btw this can be structured/separated a bid by adding empty menu entries with separator lines, like we have then in some other menus.
|
Output : [Auto] Auto is not needed as there are some things we are unable to control. If a user Sets it to SDTV and then Plugs in a HDMI cable it will default to HDMI I hate the idea of disabling a screen even if it overrides the other because users might end up in a no screen working land all because of a setting left disabled/enabled Also the amount of logic needed in code to run all the checks for is it HDMI-1/HDMI-0/SDTV/DIS/ When the user plugs in a device that is detected and it is a primary device it should boot on that screen if its working. Suggestion/Example:
*** User Selects, Output : [HDMI] and gets this screen
*** User Selects, 2 : SDTV and gets
*** User Selects, sdtv_mode=0 : Composite NTSC and gets yes/no menu
*** User Selects, yes and reboots *** User now gets (if no other device overrides SDTV)
*** Now user goes and plugs in a HDMI screen thats detected
As you can see users can still Disable an output but it is by choice and not automatic. this way the code works around what the user is doing. Not disabling all other outputs when a user selects a output. as for a user selecting DSI if is not detected (as DSI overrides all other video ) it would give him/her a msg like Sorry no DSI video device detected. please ensure device/cable is plugged in and device is connected properly. Please see your DSI device documentation for further help. OH man this is going to talk a while to code.......... CHALLENGE ACCEPTED |
oh and i plan one buying a DSI and DPI(GPIO) display(s) for testing |
Oh writing all this up makes me feel like I'm back on my IBM 286, creating a dos operating menu system. |
@meeki007 For HDMI at least "Forced : [On|Off]" can be added to force output in case HDMI monitor is not correctly detected. I am only wondering it this practically renders SDTV useless, even if no HDMI is attached but an SDTV monitor is, and if this overrides attached LCD as well or not 🤔. |
DSI - overrides all others so:
Next is HDMI - overides all but DSI
I know DPI(GPIO) video does not override the above but I don't know how it plays with SDTV....yet so I use lines like this to decide whats running atm
Its not too hard doing the logic for whats connected But I think you may be right to have an [on/off] switch in case the logic of auto detection fails for the video output connected |
yes it does. If you force HDMI you loose SDTV.
Oh I understand this.... your being too nice.....but if i were you I'd be thinking: I don't blame you. Lets keep it simple with [on/off] and [enable/disable] if you like. |
Yeah perhaps we can even actively disable SDTV then (with the other settings as well) and of course warn/inform and ask user to proceed. So it is more clear and SDTV state is shown as Off/disabled.
Hehe no problem, I am as well always in the middle between keeping things simple on the one hand and allowing more flexibility on the other hand. This also drove me crazy with the audio menu, as on a certain level one cannot have both and has to decide 😄. |
Now this should be getting closer to what you want. Every thing is enabled on purpose. Under Settings things like: SDTV , Colourburst LCD stuff off of main menu and into their sections like rotation and brightness etc I'm going to be without internet Tuesday - Thursday as I make my way back to Detroit as my vacation ends Thank you for working with me on this stuff. I just want to get it right m8 |
Working on it today and tomorrow (sat/sun) sorry had to work last weekend. |
Status: WIP
[ ] audio override menuAdmin edit: We'll handle this in a separate PR for audio options reworkReference: #2939