Closed kobbled closed 5 years ago
Thanks for the PR.
From a quick look it's not clear to me how this is different from using --core ID
in combination with -w
?
One reason the support version is currently hard-coded in the build script is because it's very likely that different controller versions require different versions of libraries.
Versions of Karel < 7.70 have quite a few bits missing or functions / routines behave slightly differently or have lost/gained arguments. Versions > 8.30 have a few new routines that cannot be used on older systems.
Simply overriding the core version in a build script in those cases will typically not work, as paths to libraries have to be updated, or it might even not be needed, as different versions of projects use different workspaces altoghether.
Compare this to CMake, where regeneration of the build script is chosen over runtime adaptation of it.
Note: the above does not mean that your proposed change is not something that should be considered for inclusion into rossum
. I just wanted to provide some context/rationale for why rossum
behaves the way it does.
I agree with this, but I will try to highlight some of the issues I was having and get your thoughts:
The robot.ini file poses a lot of issues. If the paths in the robot.ini file are non existent it will not build regardless of what options are in rossum or ktransw, including the path to the roboguide robot. since ktransw is in quite mode I wanted to include error handling to catch problems with the robot.ini file in rossum, or else the user doesn't know what is going on.
I need to double check this to confirm that the /ver flag is ktransw is working properly. There were instances I was getting different support version files, and it was building from a different ktrans version.
I also wanted to get your thoughts on just using the robot.ini file for locating files and executables. The search functions you have included are nice and helpful, but if there are issues with how roboguide is installed, users will have issues. For instance I installed roboguide on a D: drive and I was running into problems pretty quick. Or just copying the winolpc bin file, support files and robot file onto a computer without roboguide installed was problematic. Forcing users to make sure there robot.ini file is correct, search problems are up to them, and it will cut down the amount of code in rossum dramatically.
The robot.ini file poses a lot of issues. If the paths in the robot.ini file are non existent it will not build regardless of what options are in rossum or ktransw, including the path to the roboguide robot.
Yes, that would indeed be an option.
As I typically ignore Roboguide in my development setups, I only ever used "empty" robot.ini
files, so never ran into this.
since ktransw is in quite mode I wanted to include error handling to catch problems with the robot.ini file in rossum, or else the user doesn't know what is going on.
It might be good to check if there is consistency between the info in robot.ini
and what is passed to rossum
(although we shouldn't lose the option of overriding that). - Edit: #24.
I need to double check this to confirm that the /ver flag is ktransw is working properly.
It should. It's not even a flag for ktransw
, it's passed directly to ktrans
(as it's a flag with a /
).
There were instances I was getting different support version files, and it was building from a different ktrans version.
That would be a bug (or at least undesirable behaviour). If you have a reproducable example of that I would appreciate an issue over at gavanderhoorn/ktransw_py/issues.
ok. thank you for looking at this. I was mainly just wanting to see your thoughts on the matter. I do not expect you to merge this. I do think its a good idea to parse the robot.ini file and check the paths in the file though. Conflicts do throw errors. I never considered just using a blank robot.ini file... I assumed that wouldn't work with how strict roboguide is about it.
I also wanted to get your thoughts on just using the robot.ini file for locating files and executables.
It might make sense to integrate robot.ini
better anyway, as it's going to be needed for #1 (tp files).
The search functions you have included are nice and helpful, but if there are issues with how roboguide is installed, users will have issues. For instance I installed roboguide on a D: drive and I was running into problems pretty quick. Or just copying the winolpc bin file, support files and robot file onto a computer without roboguide installed was problematic. Forcing users to make sure there robot.ini file is correct, search problems are up to them, and it will cut down the amount of code in rossum dramatically.
So I understand what you're saying, but not having Roboguide or WinOLPC installed on a machine would mean that you don't have a license to use ktrans
. Or at least, not for that particular machine. I realise that ktrans
works without a license (only a few registry entries are needed actually if you want to make things nice, but it works without those as well), but I'm not sure I should be facilitating that with ktransw
.
Currently rossum
allows you to specify the location of both ktrans
(with --ktrans
) and ktransw
(using --ktransw
) as args. You can also specify the location of the support files (using --support
).
It would be a longer command line (and I haven't tested it), but I believe rossum
should not start looking for Roboguide if you specify those args (or at least should not exit with an error; it if does, that would be a bug). These lines are involved in deciding whether not being able to find Roboguide should be treated as an error, or whether we can ignore that as the user has supplied the required CLAs to be able to continue.
ok. thank you for looking at this. I was mainly just wanting to see your thoughts on the matter.
An issue would also have worked ;)
But I like the PR, as I can now see what you've done.
I do not expect you to merge this. I do think its a good idea to parse the robot.ini file and check the paths in the file though. Conflicts do throw errors.
Yes, that I would agree with.
I never considered just using a blank robot.ini file... I assumed that wouldn't work with how strict roboguide is about it.
Well, it's not completely empty, but almost (from here):
[WinOLPC_Util]
Robot=.
After some testing I found that for compiling Karel at least, this is what is needed to keep ktrans
happy. maketp
does not like it though (as I wrote in https://github.com/gavanderhoorn/rossum/pull/23#issuecomment-480745352).
Actually thinking about it rossum was never finding the roboguide reg keys on my computer. I dont know if they changed for v9 or something, but I have never looked at the output and it has found them. Want me to open an issue on this?
Actually thinking about it rossum was never finding the roboguide reg keys on my computer. I dont know if they changed for v9 or something, but I have never looked at the output and it has found them. Want me to open an issue on this?
yes, please do.
@kobbled: did we cover all your questions / the issues you encountered using rossum
with your specific setup?
I'm not against extending / changing the way rossum
finds the necessary tools (as it's far from perfect), so please open new issues if you encounter anything that you feel should behave differently.
Issue with running rossum for multiple robot controller version, where robot.ini file was not being taken into account. Environment variable for version was not efficient if running for multiple controller versions. Problem still arises for building same package for multiple controller with different versions, but an override argument is added to be able to control this with rossum options. I am currently just having multiple robot.ini files for each controller and specifying them with -r ../path/to/robot.ini during build.