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

DietPi-Config | [RPi] Display Options: Rework to add supported HDMI modes based on auto-detection #2946

Closed
wants to merge 2 commits into from

Conversation

meeki007
Copy link
Contributor

@meeki007 meeki007 commented Jul 1, 2019

  • 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

Status: WIP

  • Array and basic usage finished
  • user menus and response to selections
  • restart for changes made
  • [ ] audio override menu Admin edit: We'll handle this in a separate PR for audio options rework

Reference: #2939

- 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
@MichaIng MichaIng requested review from MichaIng and Fourdee July 2, 2019 10:26
@MichaIng MichaIng added this to the v6.26 milestone Jul 2, 2019
@@ -822,160 +822,441 @@ A long (or insufficiently manufactured) cable may required a higher boost settin

fi

# RPi
elif (( $G_HW_MODEL < 10 )); then
Copy link
Owner

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
Copy link
Owner

@MichaIng MichaIng Jul 2, 2019

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
Copy link
Owner

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'
Copy link
Owner

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.

@MichaIng
Copy link
Owner

MichaIng commented Jul 2, 2019

@meeki007
Many thanks for this!! I will help/test/implement it after v6.25 release.

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 3, 2019

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 :)

@MichaIng MichaIng changed the title Beta DietPi-Config | [RPi] Display Options: Rework to add supported HDMI modes based on auto-detection Jul 3, 2019

)
if [ "$DMT_detected" != '0' ]] || [[ "$CEA_detected" != '0' ]]; then
Copy link
Owner

@MichaIng MichaIng Jul 3, 2019

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 ))

@MichaIng
Copy link
Owner

MichaIng commented Jul 3, 2019

General idea about applying the values:
Creating two arrays while looping through detected supported modes:
e.g. aMODE[0]='DTM 2'
and e.g. aRESOLUTION[0]='1024x768'
and then the whiptail array with:
G_WHIP_MENU_ARRAY=( $i "${aMODE[0]}: ${aRESOLUTION[0]}" ... )
To apply setting, then ${aMODE[$G_WHIP_RETURNED_VALUE]} can be checked for DTM/CEA and remove first 4 chars to get the mode. And ${aRESOLUTION[$G_WHIP_RETURNED_VALUE]} can be checked for framebuffer sizes.
aMODE perhaps can be easier created based on tvservice output, not sure about the format, have to setup a Pi first with VHCI enabled 😉.

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 8, 2019

pi model 4 just showed up. Waiting on HDMI micro cables now.
Just wanted to check back and let you know im still working on this

@MichaIng
Copy link
Owner

MichaIng commented Jul 9, 2019

@meeki007
Please tell me when I should help or commit here. I just don't want to mess with your ideas for now (until you ask to) and instead do a start with the sound card selection menu 😃.

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 9, 2019

@MichaIng

I like your idea of

For RPi4 it then further needs to be divided [Headless] [SDTV/Composite] [LCD Panel] [HDMI 1] [HDMI 2] [HDMI 1+2] or something like that.

My extra cheep Chinese 7in hdmi display for a car dash install will not auto detect but my LG in my room detects fine.
I'm not going to unmount my Living room display to test or I'll face the wrath of my girl
I'm playing with code and the [HDMI:*] filter in config.txt
getting the warm a fuzzy

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 :)

@MichaIng
Copy link
Owner

I'm playing with code and the [HDMI:*] filter in config.txt

I would go with e.g. hdmi_group:<port>=<value> on RPi4, since this allows to add it anywhere in the file instead of beneath the [HDMI:<port>] filter line, which makes it more script friendly. On non-RPi4 the port can be just left out (since it defaults to 0) and on RPi4, when user selected either HDMI port 2 or both, the <command>:<port> entries just need to be added additionally and can be identified/removed easily without the need to check under which filter line it stays.

Ref: https://www.raspberrypi.org/documentation/configuration/config-txt/video.md

@meeki007
Copy link
Contributor Author

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.

@meeki007
Copy link
Contributor Author

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

@MichaIng
Copy link
Owner

MichaIng commented Jul 21, 2019

@meeki007
I just recognised that you opened the PR against beta branch. Please reopen against dev branch. I will resolve conflicts if apparent.

EDIT: Ah I was able to rebase, so everything is fine 🙂.

@MichaIng MichaIng changed the base branch from beta to dev July 21, 2019 20:23
@meeki007
Copy link
Contributor Author

@MichaIng

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

@meeki007
Copy link
Contributor Author

@MichaIng
need help with G_WHIP
besides putting in a counter how do i create a user msg that does not loop endlessly
am i missing something here with G_WHIP_MSG

#User has HDMI plugged in and has Composite Video enabled
#inform user that Composite Video and HDMI cannot run at the same time
if [[ ${config_txt[*]} != *'#sdtv_mode'* ]] && [[ ${config_txt[*]} == *'sdtv_mode'* ]] && [ "${tvservice_l_connected_detected[0]}" -ge "1" ]; then
	G_WHIP_MSG 'Dual Video Conflict: HDMI and Composite Video cannot run simultaneously.\n\nWill always default to HDMI if plugged in.\nTo run in Composite Video Mode Please unplug all HDMI cables and restart device/nTo Disable this msg please disable Composite Video '
fi

@meeki007
Copy link
Contributor Author

as promised Here is the new sub menu screen shot of it working :)

preMenu

Need to add manual resolutions and openGL under HDMI menu
This has been a nightmare of love and fun but still broke my brain a few times

@MichaIng
Copy link
Owner

@meeki007
Very nice work!

besides putting in a counter how do i create a user msg that does not loop endlessly

Why not turning it into a G_WHIP_YESNO:

  • If user enables HDMI while SDTV/composite is enabled, ask if SDTV should be disabled to continue or cancel.
  • If user enables SDTV while HDMI is enabled, ask if HDMI should be disabled to continue or cancel.

More cleaned up would be probably the following:

  • Make HDMI/SDTV a single menu entry to choose between one of them. So each selection will automatically disable the other one. The config.txt entries anyway do not allow if differently.
  • Make the resolution choice a separate menu that only shows up if HDMI is selected. On SDTV apply always the default SDTV resolution.
  • If headless mode is enabled, it would be nice if all other menu entries would be hidden. So one is not confused by several options that anyway have no effect or could even conflict with headless more, e.g. produce boot error message when related hardware is not loaded but a related setting is attempted to be applied.

@meeki007
Copy link
Contributor Author

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

@meeki007
Copy link
Contributor Author

@MichaIng
Taking today to work on optimizations in my code and old code before me

Old Code optimizations
why use grep to call config.txt so many times

local framebuffer_x=$(grep -m1 '^[[:blank:]]*framebuffer_width=' /DietPi/config.txt || vcgencmd get_config framebuffer_width)
framebuffer_x=${framebuffer_x#*=}; framebuffer_x=${framebuffer_x:-0}
local framebuffer_y=$(grep -m1 '^[[:blank:]]*framebuffer_height=' /DietPi/config.txt || vcgencmd get_config framebuffer_height)
framebuffer_y=${framebuffer_y#*=}; framebuffer_y=${framebuffer_y:-0}
local current_value=$(grep -m1 '^[[:blank:]]*dtoverlay=vc4-' /DietPi/config.txt | sed 's/^[^=]*=//') #OpenGL check 1st

when it can be put in an array to be called from

# config.txt into an array, used to check settings enabled or not
mapfile -t config_txt < "/home/dietpi/config.txt"

Optimizing array counts and such. way faster to use:
declare -i i; i+=1 [ 0m0.492s ]
vs 'let' i was using
let "i=i+1" [ 0m1.116s ]

Other optimizations as well.

Got tired of writing menu code that is not as fast a possible

:)

@MichaIng
Copy link
Owner

@meeki007
Good idea. For dietpi.txt I had the idea to make it source-able. Only the automated firstrun install IDs need to be changed and at best single quotes around all non-integer values.
This way one can call . /DietPi/dietpi.txt one time to have all included variables loaded. And I bet this call is faster then one single grep 😉.

Okay nope one grep scrape is slightly faster:

2019-07-26 22:28:10 root@micha:/run# ( time for i in {1..100}; do source /DietPi/dietpi.txt &> /dev/null; done; time for i in {1..100}; do grep -m1 '^[[:blank:]]*AUTO_SETUP_HEADLESS=' /DietPi/dietpi.txt | sed 's/^[^=]*=//' &> /dev/null; done )

real    0m1.434s
user    0m1.300s
sys     0m0.262s

real    0m1.381s
user    0m0.753s
sys     0m1.858s

@meeki007
Copy link
Contributor Author

@MichaIng

Well guess it comes down to how many greps vs iterating through an array.

I've also found its faster to

for element in "${config_txt[@]}"; do

and store a value to variable than to.....

elif [[ ${config_txt[*]} != *'#hdmi_ignore_hotplug='* ]] && [[ ${config_txt[*]} == *'hdmi_ignore_hotplug=1'* ]] && [[ ${config_txt[*]} != *'#hdmi_ignore_composite='* ]] && [[ ${config_txt[*]} == *'hdmi_ignore_composite=1'* ]]; then
				  is_headless=true

@meeki007
Copy link
Contributor Author

@MichaIng

also i'm dealing with possible user input failures

What if someone edits the config like this

# Set the following two settings to "1" to disable video output and framebuffer completely.
# NB: This will lead to some error messages on boot, which can be safely ignored.
hdmi_ignore_hotplug=1
#hdmi_ignore_hotplug=0

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

@MichaIng
Copy link
Owner

@meeki007
I would ignore all commented lines. Only read/check non-commented lines, otherwise assume default.

G_CONFIG_INJECT 'hdmi_ignore_hotplug=' 'hdmi_ignore_hotplug=1' /DietPi/config.txt

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.

@meeki007
Copy link
Contributor Author

@MichaIng

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!!!

@meeki007
Copy link
Contributor Author

@MichaIng

however this is working nicely

				###[ Settings Check ]###
				for element in "${config_txt[@]}"; do
				  # [ headless check ] #
				  # hdmi_ignore_hotplug=1
				  if [[ $element == 'hdmi_ignore_hotplug=1'* ]]; then
				    hdmi_ignore_hotplug_1=enabled
				  elif [[ $hdmi_ignore_hotplug_1 != 'enabled' ]] && [[ $element == '#hdmi_ignore_hotplug='* ]]; then
				    hdmi_ignore_hotplug_1=disabled
				  fi
				  # hdmi_ignore_composite=1
				  if [[ $element == 'hdmi_ignore_composite=1'* ]]; then
				    hdmi_ignore_composite_1=enabled
				  elif [[ $hdmi_ignore_composite_1 != 'enabled' ]] && [[ $element == '#hdmi_ignore_composite='* ]]; then
				    hdmi_ignore_composite_1=disabled
				  fi
				done

and i can keep adding checks to it as needed so it only runs through the array one time

@meeki007
Copy link
Contributor Author

@MichaIng

I would ignore all commented lines. Only read/check non-commented lines, otherwise assume default.

I got ya now!!! way less verbage

############## CHANGE THIS LOCATION BACK AFTER TESTING
mapfile -t config_txt < "/home/dietpi/config.txt"

###[ Settings Check ]###
for element in "${config_txt[@]}"; do
  # hdmi_ignore_hotplug=1
  if [[ $element == 'hdmi_ignore_hotplug=1'* ]]; then
    hdmi_ignore_hotplug_1=enabled
  fi
  # hdmi_ignore_composite=1
  if [[ $element == 'hdmi_ignore_composite=1'* ]]; then
    hdmi_ignore_composite_1=enabled
  fi
done

# [ headless check ] #
if [[ $hdmi_ignore_hotplug_1 == 'enabled' ]] && [[ $hdmi_ignore_composite_1 == 'enabled' ]]; then
  headless=enabled
else
  headless=disabled

@meeki007
Copy link
Contributor Author

@MichaIng

I can't murge the HDMI and SDTV
HDMI has too many things going on in pi4 now:
hdmi_enable_4kp60
hdmi 0 or 1
etc
then under that menu you can set the resolutions.

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?
Also I need a better sentence than "Current Display Output" it just sound funny to me for the current mode used.

menu_test12

and if you enable headless this happens now :)

menu_test14

This is the code in case i get hit by a bus

###[ config.txt inport ]###
# config.txt into an array, used to check settings enabled or not
############## CHANGE THIS LOCATION BACK AFTER TESTING
mapfile -t config_txt < "/home/dietpi/config.txt"

#[ Settings Check ]#
for element in "${config_txt[@]}"; do
  # hdmi_ignore_hotplug=1
  if [[ $element == 'hdmi_ignore_hotplug=1'* ]]; then
    RPi_hdmi_ignore_hotplug_1=enabled
  fi
  # hdmi_ignore_composite=1
  if [[ $element == 'hdmi_ignore_composite=1'* ]]; then
    RPi_hdmi_ignore_composite_1=enabled
  fi
  # hdmi_blanking=1
  if [[ $element == 'hdmi_blanking=1'* ]]; then
    RPi_hdmi_blanking_1=enabled
  fi

  # sdtv_mode
  if [[ $element == 'sdtv_mode'* ]]; then
    RPi_sdtv_mode=enabled
  fi
  # enable_tvout_1
  if [[ $element == 'enable_tvout=1'* ]]; then
    RPi_enable_tvout_1=enabled
  fi
  #[ VC4 - check ]#
  if [[ $element == 'dtoverlay=vc4'* ]]; then
    RPi_VC4=enabled
  fi
done

#[ headless check ]#
if [[ $RPi_hdmi_ignore_hotplug_1 == 'enabled' ]] && [[ $RPi_hdmi_ignore_composite_1 == 'enabled' ]] && [[ $RPi_hdmi_blanking_1 == 'enabled' ]]; then
  RPi_headless=enabled
fi
#[ sdtv check ]#
if [[ $RPi_sdtv_mode == 'enabled' ]] && [[ $RPi_enable_tvout_1 == 'enabled' ]]; then
  RPi_sdtv=enabled
fi
#[ DSI check ]#
# output of vcgencmd get_config display_default_lcd into an var, used to check if dispalys are Detected/Connected
value_of_vcgencmd_get_config_display_default_lcd=$(vcgencmd get_config display_default_lcd)
if [[ $value_of_vcgencmd_get_config_display_default_lcd == 'display_default_lcd=1' ]]; then
RPi_DSI=enabled
fi

###[ tvservice -l inport ]###
# output of tvservice -l into an array, used to check if dispalys are Detected/Connected
readarray -t output_of_tvservice_l_array <<< "$(tvservice -l)" #see if hdmi connected/detected, Save output of tvservice -l, Add detected displays to array

#[ Get number of displays connected/detected ]#
tvservice_l_connected_detected=${output_of_tvservice_l_array[0]::1}  # Return the first character
#[ HDMI check]# & Add detected displays to attached devices array(s) if they were detected
if [ "${tvservice_l_connected_detected[0]}" -ge "1" ]; then
  RPi_HDMI=enabled
  LineNumber=1 #Usable info starts at line 1
  Nullcheck=${output_of_tvservice_l_array[$LineNumber]} #Check when we reach the end of the array
  until [ ! "$Nullcheck" ] || [ $LineNumber -gt 100 ] ; do #make sure it stops even if array is huge

    #format to get just the numbers
    HDMI_number_attached_device[$LineNumber]=${output_of_tvservice_l_array[$LineNumber]:(-1)}  # keep the last character of the string
    Display_number_attached_device[$LineNumber]=${output_of_tvservice_l_array[$LineNumber]%%,*}  # retain the part before character ,
    Display_number_attached_device[$LineNumber]=${Display_number_attached_device[$LineNumber]:(-1)}  # keep the last character of the string

    let LineNumber=LineNumber+1
    Nullcheck=${output_of_tvservice_l_array[$LineNumber]}
  done
fi

#***# Resolution Submenu Level _1_ #***#
#[ HDMI Connected/Detected ]#
if [[ $RPi_HDMI == 'enabled' ]] && [[ $RPi_DSI != 'enabled' ]]; then
  #Add HDMI to MENU
  G_WHIP_MENU_ARRAY=('HDMI' ': Current Display Output')
else
  G_WHIP_MENU_ARRAY=('HDMI' ': ')
fi
#Add SDTV to MENU
if [[ $RPi_sdtv == 'enabled' ]] && [ "${tvservice_l_connected_detected[0]}" -eq "0" ] && [[ $RPi_DSI != 'enabled' ]]; then
  G_WHIP_MENU_ARRAY+=('SDTV' ': Current Display Output')
else
  G_WHIP_MENU_ARRAY+=('SDTV' ': ')
fi
#Add DSI to MENU
if [ "${tvservice_l_connected_detected[0]}" -eq "0" ] && [[ $RPi_DSI == 'enabled' ]]; then
  G_WHIP_MENU_ARRAY+=('DSI' ': Current Display Output')
else
  G_WHIP_MENU_ARRAY+=('DSI' ': ')
fi

#Add DPI to MENU
#COMMING_SOON!!!!!!!
G_WHIP_MENU_ARRAY+=('DPI' ': ')

#Add VC4 Display Driver to MENU
if [[ $RPi_VC4 == 'enabled' ]]; then
  G_WHIP_MENU_ARRAY+=('VC4-driver' ': Enabled')
else
  G_WHIP_MENU_ARRAY+=('VC4-driver' ': Disabled')
fi

if [[ $RPi_headless == 'enabled' ]]; then
#Add Headless Enabled/Disabled to MENU
  G_WHIP_MENU_ARRAY=('Headless' ': Enabled')
else
  G_WHIP_MENU_ARRAY+=('Headless' ': Disabled')
fi

@MichaIng
Copy link
Owner

MichaIng commented Jul 27, 2019

@meeki007
Indeed merging HDMI and SDTV modes/resolutions does not make much sense, sespecially since in case of auto/hotplug detection, both need to be set separately. This is currently not possible as well (DietPi stable code I mean), but makes sense. So when user attaches a HDMI cable, the Pi switches with a chosen resolution automatically instead of having somehow SDTV resolution + framebuffer limit set.

I have again another, perhaps slightly cleaner menu structure idea 😅:

Output : [HDMI/SDTV/Auto/Headless]
  • This switches SDTV and/or HDMI output and/or hotplug settings, dependent on choice.

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?)
On RPi4 an additional entry is required to switch "HDMI port" between 1/2/both.
In case of both, each HDMI entry needs to be shown doubled, allowing to switch settings for each HDMI port individually.

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.
E.g.

Output : [Auto]
●─ HDMI Options ─────
HDMI resolution
HDMI boost
...
●─ SDTV Options ─────
SDTV mode
SDTV aspect ratio
...
●─ General Options ────
VC4 driver
Overscan
...

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 27, 2019

@MichaIng

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
then if you plug in a DSI cable to a screen it will default to that.

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/
and make sure the config file has the enables/disables would be a nightmare

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 Plugs in a HDMI (pick any supported interface SDTV etc) and boots for the first time

Please select an Output or edit $current_output Options  

Output : [HDMI]
●─ HDMI Options ─────
HDMI resolution
HDMI boost
●─ General Options ────
VC4 driver
Overscan

*** User Selects, Output : [HDMI] and gets this screen

1 : HDMI
2 : SDTV
3 : DSI
4 : Headless
5 : Advanced Options
6 : etc foo foo

*** User Selects, 2 : SDTV and gets

sdtv_mode=0 : Composite NTSC
sdtv_mode=1 : Composite Japanese NTSC
sdtv_mode=2 : Composite PAL
sdtv_mode=3 : Composite Brazilian PAL

*** User Selects, sdtv_mode=0 : Composite NTSC and gets yes/no menu

Set Video Output to: sdtv_mode=0 : Composite NTSC?

Please Disconnect any HDMI, DSI video cables as they will override SDTV video as the default device.

A reboot will be required for settings to take effect

*** User Selects, yes and reboots

*** User now gets (if no other device overrides SDTV)

Output : [SDTV]
●─ SDTV Options ─────
SDTV mode
SDTV aspect ratio
●─ General Options ────
VC4 driver

*** Now user goes and plugs in a HDMI screen thats detected

Warning: SDTV is enabled. Please unplug $current_output or disable $current_output under Advanced Options found in the Output: menu if you wish to use SDTV.
To stop this warning if you wish to continue using $current_output please Disable: [SDTV]

Output  : [HDMI]
Disable : [SDTV]
●─ HDMI Options ─────
HDMI resolution
HDMI boost
●─ General Options ────
VC4 driver
Overscan

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

@meeki007
Copy link
Contributor Author

oh and i plan one buying a DSI and DPI(GPIO) display(s) for testing

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 27, 2019

Oh writing all this up makes me feel like I'm back on my IBM 286, creating a dos operating menu system.
The only difference is I'm not worried about the too few bytes I have for comments in code.
Nice not having a book and pencil with line codes for comments and errors.

@MichaIng
Copy link
Owner

MichaIng commented Jul 27, 2019

@meeki007
Hmm, how do you detect if either HDMI and/or SDTV and/or DSI and/or DPI is connected or not? Basically for me this sounds like all settings need to be shown in every case where headless is disabled.
Then every sub menu can contain a "Status : [On|Off]" entry. HDMI and SDTV have settings for enable/disable it, DSI + DPI require certain dtoverlays to be added.
Okay is perhaps a better alternative that has more flexibility. But it makes the menu more overall complex as well, looses some simplicity.
Forcing any output is possible as well, as there are "ignore" settings for every output type so that only one can be left. That is not too hard to check as well.

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 🤔.

@meeki007
Copy link
Contributor Author

@MichaIng

how do you detect if either HDMI and/or SDTV and/or DSI and/or DPI is connected or not

DSI - overrides all others so:

#[ DSI check ]#
# output of vcgencmd get_config display_default_lcd into an var, used to check if dispalys are Detected/Connected
value_of_vcgencmd_get_config_display_default_lcd=$(vcgencmd get_config display_default_lcd)
if [[ $value_of_vcgencmd_get_config_display_default_lcd == 'display_default_lcd=1' ]]; then
RPi_DSI=enabled
fi

Next is HDMI - overides all but DSI

###[ tvservice -l inport ]###
# output of tvservice -l into an array, used to check if dispalys are Detected/Connected
readarray -t output_of_tvservice_l_array <<< "$(tvservice -l)" #see if hdmi connected/detected, Save output of tvservice -l, Add detected displays to array

#[ Get number of displays connected/detected ]#
tvservice_l_connected_detected=${output_of_tvservice_l_array[0]::1}  # Return the first character
#[ HDMI check]# & Add detected displays to attached devices array(s) if they were detected
if [ "${tvservice_l_connected_detected[0]}" -ge "1" ]; then
  RPi_HDMI=enabled

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

#Add SDTV to MENU
if [[ $RPi_sdtv == 'enabled' ]] && [ "${tvservice_l_connected_detected[0]}" -eq "0" ] && [[ $RPi_DSI != 'enabled' ]]; then
  G_WHIP_MENU_ARRAY+=('SDTV' ': Current Display Output')
else
  G_WHIP_MENU_ARRAY+=('SDTV' ': ')
fi

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

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 27, 2019

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

yes it does. If you force HDMI you loose SDTV.
might want a warning that HDMI is Forced and SDTV is enabled. Let the user know why his rca to TV is failing

But it makes the menu more overall complex as well, looses some simplicity.

Oh I understand this.... your being too nice.....but if i were you I'd be thinking:
"who's this meeki007 guy?"
"is he going to BLOB a ton of code that works now and later no one can dig through when things change and meeki007 can't be found"

I don't blame you. Lets keep it simple with [on/off] and [enable/disable] if you like.

@meeki007 meeki007 closed this Jul 27, 2019
@meeki007 meeki007 deleted the beta branch July 27, 2019 18:54
@MichaIng
Copy link
Owner

MichaIng commented Jul 27, 2019

might want a warning that HDMI is Forced and SDTV is enabled. Let the user know why his rca to TV is failing

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.

I don't blame you. Lets keep it simple with [on/off] and [enable/disable] if you like.

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 😄.
Anything you do will be an enhancement and I trust you to find a good solution. Many thanks btw for doing all this coding and testing effort! Most importantly the supported HDMI mode auto-detection and RPi4 compatibility is in when we release, everything else is fine tuning IMO.
We or me can always adjust based on feedback later on.

@meeki007
Copy link
Contributor Author

meeki007 commented Jul 28, 2019

Now this should be getting closer to what you want. Every thing is enabled on purpose.

menu44

Under Settings things like:
HDMI , 4k @ 60Hz, Rotation ,Boost

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
Expect more code next weekend

Thank you for working with me on this stuff. I just want to get it right m8

@meeki007
Copy link
Contributor Author

Working on it today and tomorrow (sat/sun) sorry had to work last weekend.
expect more screen shots and code late Sunday night EST

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

Successfully merging this pull request may close these issues.

2 participants