Danitegue / PCBRW_OLD

A repository with all the needed things to run and control BREWER instruments software, under a Pyhton enviroment.
0 stars 0 forks source link

Dependencies not all needed #2

Open robhagemans opened 6 years ago

robhagemans commented 6 years ago

Hi Daniel,

Just a quick remark on dependencies: for the current PC-BASIC master you don't need all of

pip install pypiwin32 pysdl2 numpy pygame pyaudio pyserial

pypiwin32 is no longer needed at all and pygame and pyaudio are only needed if you want to use the PyGame interface and the PortAudio sound engine, respectively. If you use SDL2 neither of those are necessary.

In fact if you just use it through the ANSI terminal the following should suffice:

pip install pyserial

as numpy and pysdl2 are only needed for the graphical interface. Could save your users some hassle...

If you don't need them to have your logging additions then they could also just do pip install pcbasic which would draw in the dependencies automatically and install PC-BASIC as a Python module.

Rob

Danitegue commented 5 years ago

Hi Rob,

I have deleted pywin32 in the readme as suggested.

Since I don't know the platforms in which this big ship is going to be tested, I prefeer to leave for now the needed dependencies for pygame, and for sdl2. I know there are many people interested in switching to linux some brewer instruments... so better to leave these options available for now.

Now PCBASIC and PCBRW are identical except for the extended logging. Is a pitty that you don't want to include the few logging additions I have in PCBRW in the current branch of PCBASIC, I think it would be easy to add them, perhaps implemented in a more efficient way, and enabled as an option by extra arguments in the launchers. Is really helpful to see what f...ing is doing the huge BASIC code in each moment, by simply seeing in the log which program is going to be executed, which files are being opened, or what is being sent/received through the shell commands. (useful also to discover why the files are not copied/appended in the same way than in the original COMMAND.COM of gw!). Also It was very easy to me to build the brewer_simulator by seeing the com port communications in the logs.

I'm sure the brewer community needs these extended logs for doing research, experiments, and also for making debug in their own modified BASIC codes of the brewer software.

For all those people that does not need any of these info in the logs, of course is much easier to install PCBASIC as you suggest with pip install pcbasic.

robhagemans commented 5 years ago

Hi Daniel,

Cheers - note that Linux can use the ANSI terminal as well as (in fact much more easily than) Windows, and SDL2 is also available on all platforms (e.g. on Ubuntu Linux it's easily available with apt install libsdl2-2.0.0 and you won't need to install any further DLLs). But no harm in including the sdl and pygame packages. I'm not actively developing the Pygame interface anymore so it may at some point start working less well than SDL. I'll probably mark the Pygame option as deprecated one of these days.

I'll give the logging/tracing program activity some more thought but I don't want to overflow my logs with tons of lines that I don't need, drowning out what's important to me, and I don't want numerous logging commands in the code as I find that (for me) it makes the code harder to read and maintain. As to adding more command-line options to support these logging settings, one of my targets was actually to reduce the number of command-line options and simplify them, again to reduce maintenance load (since every option, in theory, at least doubles the number of possible code paths...). But there may be a way around these issues. I have a lot less time to spend on this thing than before, unfortunately.

robhagemans commented 5 years ago

I've added some debug-level logging for serial port open/close and read/write and shell commands, which I think is the main additional logging you have. Have a look if this meets your needs, it's currently in the topic-logging branch but I'll probably merge it into master after some more testing.

I saw you also added some code for tracing variables but I would suggest you use the _WATCH extension statement for this, which is enabled in debug mode. For example, _WATCH "A$" will follow the value of a$. Additionally, _TRACE will print the current line number to the log file. Further logging functionality could be implemented through the debug extension - or you could maintain your own additional extension module, which would be easier than maintaining a fork.

Danitegue commented 5 years ago

Thanks Rob! These features are really useful. But how could we enable/disable the com port read/write messages in your version? This would be enabled for research purposes, but disabled for regular operation. (Otherwise the logs would be huge).

I will test it, and If the logging is enough, I will modify the PCBRW readme.md as soon I have a little bit of time, in order to use your pip install pcbasic version, with only my already prepaired launchers for the brewer software. In this case PCBRW repo will remain only as a "how to" manual for the brewer community.

Respect the _WATCH and TRACE extensions... I have not used it yet, but by the "" I suppose it is something that has to be included in the basic code, and then it will be handled by a built in pcbasic extension function, not?. This is a problem in the point of view of retro-compatibility: once it is added a "_something" in the BASIC code, this basic code cannot be run anymore in GWBASIC nor DOSBOX. Don't you think for this specific features it would be good not to have to modify the BASIC code?: for example, if having a --trace_var ["A$", "B$"] as argument, this would activate internally the _WATCH "A$" and _WATCH "B$" without having to modify the basic code. Or, if you prefer not having more arguments, this could be done trough the .ini options file also.

El sáb., 11 ago. 2018 a las 9:28, Rob Hagemans (notifications@github.com) escribió:

I've added some debug-level logging for serial port open/close and read/write and shell commands, which I think is the main additional logging you have. Have a look if this meets your needs, it's currently in the topic-logging branch but I'll probably merge it into master after some more testing.

I saw you also added some code for tracing variables but I would suggest you use the _WATCH extension statement for this, which is enabled in debug mode. For example, _WATCH "A$" will follow the value of a$. Additionally, _TRACE will print the current line number to the log file. Further logging functionality could be implemented through the debug extension - or you could maintain your own additional extension module, which would be easier than maintaining a fork.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Danitegue/PCBRW/issues/2#issuecomment-412263420, or mute the thread https://github.com/notifications/unsubscribe-auth/AYutGDR4ff8BiLAql2TNgAEucWiK53u8ks5uPqPCgaJpZM4TeXm7 .

--

Daniel Santana Díaz, Danitegue@gmail.com

robhagemans commented 5 years ago

Hi, these are debug-level messages, so they are only enabled if the --debug option is used.

The extension statements are indeed BASIC, but there is no need to change the code to use them: you can use the --exec option to execute them at the start of the session.For example,

pcbasic --exec='_trace:_watch "a$"'.

(I think the single quotes work on the Linux command line, on Windows you may have to experiment with command line escapes to get the quotes and spaces through correctly). This may be easier in the ini file where you won't need escapes, just

exec=_trace:_watch "a$"

On Mon, 13 Aug 2018, 12:20 Danitegue, notifications@github.com wrote:

Thanks Rob! These features are really useful. But how could we enable/disable the com port read/write messages in your version? This would be enabled for research purposes, but disabled for regular operation. (Otherwise the logs would be huge).

I will test it, and If the logging is enough, I will modify the PCBRW readme.md as soon I have a little bit of time, in order to use your pip install pcbasic version, with only my already prepaired launchers for the brewer software. In this case PCBRW repo will remain only as a "how to" manual for the brewer community.

Respect the _WATCH and TRACE extensions... I have not used it yet, but by the "" I suppose it is something that has to be included in the basic code, and then it will be handled by a built in pcbasic extension function, not?. This is a problem in the point of view of retro-compatibility: once it is added a "_something" in the BASIC code, this basic code cannot be run anymore in GWBASIC nor DOSBOX. Don't you think for this specific features it would be good not to have to modify the BASIC code?: for example, if having a --trace_var ["A$", "B$"] as argument, this would activate internally the _WATCH "A$" and _WATCH "B$" without having to modify the basic code. Or, if you prefer not having more arguments, this could be done trough the .ini options file also.

El sáb., 11 ago. 2018 a las 9:28, Rob Hagemans (<notifications@github.com

) escribió:

I've added some debug-level logging for serial port open/close and read/write and shell commands, which I think is the main additional logging you have. Have a look if this meets your needs, it's currently in the topic-logging branch but I'll probably merge it into master after some more testing.

I saw you also added some code for tracing variables but I would suggest you use the _WATCH extension statement for this, which is enabled in debug mode. For example, _WATCH "A$" will follow the value of a$. Additionally, _TRACE will print the current line number to the log file. Further logging functionality could be implemented through the debug extension - or you could maintain your own additional extension module, which would be easier than maintaining a fork.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Danitegue/PCBRW/issues/2#issuecomment-412263420, or mute the thread < https://github.com/notifications/unsubscribe-auth/AYutGDR4ff8BiLAql2TNgAEucWiK53u8ks5uPqPCgaJpZM4TeXm7

.

--

Daniel Santana Díaz, Danitegue@gmail.com

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Danitegue/PCBRW/issues/2#issuecomment-412486266, or mute the thread https://github.com/notifications/unsubscribe-auth/AHXYksdMQlGbU_0ouFxENyBmGiMbFo_dks5uQWEMgaJpZM4TeXm7 .

robhagemans commented 5 years ago

I checked - on Windows, you can use command line escapes like this: pcbasic main.asc --debug --exec="_trace:_watch \"a$\""