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:tomtom: Make sure the tomtom_plugin and tomtom_minimal contain navit.xml #875

Merged
merged 12 commits into from
Nov 8, 2019

Conversation

gefin
Copy link
Contributor

@gefin gefin commented Sep 18, 2019

the circleci build Job 15587 tomtom_plugin and tomtom_minimal dont contain a navit.xml.
also the locales Path is wrong.
Closes: #823

Fix navit.xml (tomtom480.xml) and locales path
Fix missing navit.xml (tomtom480.xml) and locale path
@gefin
Copy link
Contributor Author

gefin commented Sep 18, 2019

With this patches i get a tomtom480.xml, but the XML is not for Tomtom.
The xml dont contain layouts so we should them copy too.
Graphics are gtk, but should be SDL. The Resolution is also wrong.

I wonder because my local build environment makes a tomtom480.xml with SDL good resolution.

How to change?

@lains
Copy link
Contributor

lains commented Sep 21, 2019

It seems for tomtom, there is a dedicated XLST transformation that is applied during the build.

I had a similar issue before for platform WinCE, and it was because no proper xslt processor was found during the build from CircleCI...
From the circleCI output for tomtom_minimal (below), it seems its is the same issue:

CMake Warning at CMakeLists.txt:589 (message):
  No XSLT processor available.  You have to configure navit.xml yourself, or
  install an XSLT processor (supported:
  saxonb-xslt;saxon;saxon8;saxon-xslt;xsltproc;transform.exe).

@lains
Copy link
Contributor

lains commented Sep 21, 2019

OK, I just read in the separate issue (#823 (comment)) that you had already found out.

@gefin
Copy link
Contributor Author

gefin commented Sep 21, 2019

Yes, but i dont know how to add xslt support to ci.
A second problem is that the layouts now in extra files. In my home environment this layout files are empty.

@lains
Copy link
Contributor

lains commented Sep 22, 2019

The docker image (navit/tomtom-ng) used for build by circleci is specified here:
https://github.com/navit-gps/navit/blob/trunk/.circleci/config.yml#L182

So, I guess either the docker image should be updated to include an xslt processor, or maybe an easier way is to add a step that installs the right tool during the setup, similarly to what I did here:
https://github.com/navit-gps/navit/blob/trunk/scripts/setup_wince.sh#L7

@gefin
Copy link
Contributor Author

gefin commented Sep 22, 2019

I need help with this patch request.
How to remove the last two commits.
I was not able to add clean xslt support without affecting other platforms.

Actual knowings:
After adding xslt support to Update setup_common_requirements.sh navit.xml is built for Tomtom.
setup_tomtom_requirenments where this change should be is not called at all.
Because i was unsure i removed the change in setup_common_requirements.sh navit.xml.

Navit.xml is built without layouts. The layout-files are empty so that we need additional changes for this problem.

@gefin gefin mentioned this pull request Sep 22, 2019
@lains
Copy link
Contributor

lains commented Sep 23, 2019

Hi @gefin, the easiest way to undo your last two commits is to do a git revert on these commits.

@lains
Copy link
Contributor

lains commented Sep 23, 2019

CircleCI seems to only call scripts/setup_common_requirements.sh.

Changing setup_common_requirements.sh will probably have side effects, so you're right, your changes probably should go into setup_tomtom_requirements.sh.

And then, I would assume that this setup_tomtom_requirements.sh should be executed by CircleCI right after running setup_common_requirements.sh (this requires adding a new line inside https://github.com/navit-gps/navit/blob/trunk/.circleci/config.yml)

@lains
Copy link
Contributor

lains commented Sep 25, 2019

Tyring to implement what I suggested here...

@lains
Copy link
Contributor

lains commented Sep 25, 2019

Now xslt seems to be applied... but running the whole setup_tomtom_requirements.sh seems overkill as it downloads and builds plenty of tools that are probably already in the docker image used for build.
So the really only missing step here was:
apt-get install -y xsltproc

@lains
Copy link
Contributor

lains commented Sep 25, 2019

Moving the xslt install command inside circleci (this avoids running the full https://github.com/navit-gps/navit/blob/trunk/scripts/setup_tomtom_requirements.sh).
This lines-up the way circleci builds tomtom with what is done for Linux or Windows for example (where a bunch of apt-get install commands are run inside the environment preparation step)

I'm also keeping apt-get install -y xsltproc inside setup_tomtom_requirements.sh as well because I think it should also be installed when bringing up an environment to build for tomtom outside of CircleCI.

@gefin
Copy link
Contributor Author

gefin commented Sep 25, 2019

Now the zip contain a good navit.xml for tomtom.
Also the layout files like navit_layout_car.xml are in the zip.

The problem now is that navit_layout_car.xml are empty and navit.xml dont include it.

@lains
Copy link
Contributor

lains commented Sep 26, 2019

I give it a try...
XLST format has always been a challenge for me, but I think this should work on both included and stand-alone layout definitions.

I also updated the part of the script that copies over of XML config giles, it is now more specific which will hopefully avoid copying over non-related xml files.

@gefin, does this work better?
I guess you have a tomtom target to test this on. I am interested in knowing if the split of layouts into stand alone XML (that I developped one year ago) works on that platform. At least, it seems I broke it until now... sorry!

@gefin
Copy link
Contributor Author

gefin commented Sep 26, 2019

@lains Thank you.
The Layouts work now well on the real device.
Some gui internal icons are a little to big, but i think i am able to change this.

Need some tests.

@gefin
Copy link
Contributor Author

gefin commented Sep 26, 2019

Today real drive worked good.
I changed some lines in navit.xml because i fond no setting in the XSLT.
So the GUI internal fit better to screen:
from

<gui type="internal" enabled="yes" >

to

<gui type="internal" enabled="yes" font_size="250" icon_l="48" >

Also added 2 OSD zoom buttons:

 <osd enabled="yes"  type="button" w="60" h="60" x="-60" y="50" command="zoom_in()" src="gui_zoom_in.png"/>                                    
 <osd enabled="yes"  type="button" w="60" h="60" x="-60" y="100" command="zoom_out()" src="gui_zoom_out.png"/> 

is this possible in the XSLT?

Note : edited by @pgrandin : I added ``` around your xml code so that it would show up

@mvglasow
Copy link
Contributor

@gefin re zoom buttons, have a look at #737, where we implemented a common OSD layout across all platforms. Tomtom was the only one left out back then because the build was broken and we had no way to verify things worked as intended. The (IMHO) perfect solution would be to apply the changes from #737 to Tomtom as well, with the necessary modifications—preferably by someone who knows their way around that particular build and can test on a real device :-)

@mvglasow
Copy link
Contributor

correction to the above: as I infer from the PR, we did implement this on Tomtom in the hope that it would work, as we couldn’t test it back then…

@gefin
Copy link
Contributor Author

gefin commented Sep 27, 2019

I can test on TT 730 (480x272).

@gefin
Copy link
Contributor Author

gefin commented Sep 27, 2019

@mvglasow just checked against #737 and it look like osd_minimum is included in tomtom.xslt
After build navit.xml contain no osd lines .
The other Question is how to add icon and font size.

@metalstrolch
Copy link
Contributor

You can have a look at the "sailfish" xslt's. I think they basically do what you need. You can copy from them.

@mvglasow
Copy link
Contributor

If navit.xml contains no OSD lines, that looks like something has gone wrong. Do the other settings from tomtom.xslt take effect? If not, that would be a sign the whole XSLT is not getting applied. One way or the other, you might want to look out for warning/error messages related to XML processing.

@gefin
Copy link
Contributor Author

gefin commented Sep 28, 2019

I dont see errors or warnings.
The lines for gps source, map storage, graphic, geometry some other things are applied.

If i change something bad i see messages.
The sailfish configuration is to different for me to see where the problem is.

@gefin
Copy link
Contributor Author

gefin commented Oct 1, 2019

My changes from tomtom.xslt dont work.
With " copy-of " i get osd buttons into the xml, but i get them not enabled.

I need help with tomtom.xslt

@jkoan
Copy link
Member

jkoan commented Oct 16, 2019

My suggestion would be that we only need one configuration regarding the display size. This can be achieved by using relative sizes within osd for example. This will simplify things. Also I would like to provide another pull request wich also tryes to fix #823 but with a cpack approach. If this is okay for you. Then we can decide which solution is better.

@gefin
Copy link
Contributor Author

gefin commented Oct 16, 2019

Yes it is ok for me. We have only two relevant resolutions. 480x272 and 320x240

@aerostitch aerostitch changed the title Issue/823 fix:tomtom: Make sure the tomtom_plugin and tomtom_minimal contain navit.xml Oct 29, 2019
@jkoan
Copy link
Member

jkoan commented Nov 8, 2019

Will merge this now, and correct everything else in another PR since there are more than only on thing to work on. THX for your work

@jkoan jkoan merged commit f742fff into navit-gps:trunk Nov 8, 2019
@kortemik kortemik mentioned this pull request Jan 8, 2020
viktorgino pushed a commit that referenced this pull request Sep 22, 2020
…it.xml (#875)

* Issue/823 

Fix navit.xml (tomtom480.xml) and locales path

* Issue/823

Fix missing navit.xml (tomtom480.xml) and locale path

* add xslt support

* Update setup_common_requirements.sh

* Moving change performed in aa10e05 inside setup_tomtom_requirements.sh (and adding it to circleci process)

* Only running installation for xsltproc in circleci

* Update build_tomtom_minimal.sh

copy also the layout xml files to target

* copy also the layout xml to target

* Selecting specific xml prefixes during copy

* Fixing tomtom's xslt following split of xml config files at e70a289
jkoan pushed a commit to jkoan/navit that referenced this pull request Jun 30, 2021
…it.xml (navit-gps#875)

* Issue/823 

Fix navit.xml (tomtom480.xml) and locales path

* Issue/823

Fix missing navit.xml (tomtom480.xml) and locale path

* add xslt support

* Update setup_common_requirements.sh

* Moving change performed in aa10e05 inside setup_tomtom_requirements.sh (and adding it to circleci process)

* Only running installation for xsltproc in circleci

* Update build_tomtom_minimal.sh

copy also the layout xml files to target

* copy also the layout xml to target

* Selecting specific xml prefixes during copy

* Fixing tomtom's xslt following split of xml config files at e70a289
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.

tomtom_plugin and tomtom_minimal dont contain a navit.xml
6 participants