Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add screen mirroring in Infotainment.vspec #737
Add screen mirroring in Infotainment.vspec #737
Changes from 6 commits
bbad298
ef4ed6c
24863d3
d139b3a
02b3e85
35bd28c
3549954
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Consider using the following instead:
Although many folks tend to write "Wifi" or "WiFi", the correct spelling is "Wi-Fi": https://www.wi-fi.org/
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.
I have no strong opinion but the literals used here correspond to what we have in https://github.com/COVESA/vehicle_signal_specification/blob/master/spec/Cabin/Infotainment.vspec#L258
So if we are to change to
WI-FI
we should also consider changingVehicle.Cabin.Infotainment.SmartphoneProjection.Source
(and then comes the question on how to handle backward compatibility, just addingWI-FI
and keepWIFI
for now, but state thatWIFI
may be removed later)I suggest we discuss it at a meeting
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.
My point was mainly that if we wish the catalog to be a standard, we should ensure we use and refer to other standards. Here, the Wi-Fi alliance is using "Wi-Fi" in all its documentation. Even their domain is www.wi-fi.com.
Most people write it as Wifi, so "WIFI" should be fine. If we already have "WFI" elsewhere, I am not sure it is worth it to change to other uses.
My nitpick fits in more in the context of my interest in developing a signal catalog submission review process that looks at these kinds of checks as well as other questions we may wish to ask for each submission.
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.
Thanks for your comment.
Personally, I don't have a strong opinion on this matter either. I also agree that following the global standards is important.
However, I'm little bit confused about whether it's appropriate to use "WI-FI" for data naming. When I see the android document, "WIFI" is used as a data type name, and "Wi-Fi" is used for descriptive text in the document. (https://source.android.com/docs/core/ota/modular-system/wifi)
Typically, hyphens ('-') are not used in data type names in VSS. Therefore, how about we keep using "WIFI" as it is?
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.
I think changing "WIFI" to "WI-FI" is not too difficult since we only need to update the naming in SmartphoneProjection. More important thing is making the right decision regarding the correct naming between WIFI and WI-FI.
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.
At the VSS meeting earlier this week we agreed that keeping
WIFI
is the best option for now, as we then do not need to change the other existing signal that useWIFI
.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.
Not clear if and how this is useful. If useful, there is possibly much more information that may be useful (screen size, etc.).
Pardon my lack of knowledge, but isn't resolution a factor of what the display supports as well as what the application wishes to use or requires? If so, how do we address it here?
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.
In my opinion, screen size is not an appropriate parameter for screen mirroring configuration. This is because even within the same screen size, smartphones can support various resolutions based on user preferences. The android base smartphone also supports the configuration of your resolution in 'configuration->display->screen resolution'.
The resolution setting is actually related to the display graphics processing load of the infotainment system, and typically, it can be adjusted to suit the user's preferences.
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.
Would it be a possible way forward to clarify in the description that this signal concerns the resolution currently used, not maximum allowed resolution, possibly by changing to something like:
"Display resolution currently used for the mirrored content"
This off course does not prevent that more signals are added later if needed, like attributes for screen size, supported resolutions and so on, but that could be added later if/when needed.