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

Various improvements and bug-fixes #91

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

GlassesToCSharp
Copy link

This PR contains a variety of improvements and bug-fixes that were found when running on a Raspberry Pi 3 Model B (B+?).

Major Changes

Replaced Flask server with NodeJS

This change follows after the RPi was unable to run sox with an instance of the Flask webserver. It was only after removing the thread running the flask server that recording and processing worked on the RPi. This was raised here.

The alternative was to use a NodeJS server. Why use NodeJS instead of fixing the Flask instance?

  • I have no experience with Flask, so fixing the above issue was only going to take a long time and make things worse.
  • I have another RPi home server running NodeJS, and it runs very well.
  • I didn't like having the same script running signal processing and a webserver instance.
  • Threads.
  • If I want to make changes to the signal processing (Python and/or Bash code), I don't need to close the webserver to amend and update the Python code.

Using NodeJS, I now have two terminals: one running the web server instance, and the other running AutoWX2. Interupting one will not affect the other.

To set up a NodeJS server on a Raspberry Pi, there is a very good tutorial by thisDaveJ that takes through all the steps to setting it up.

subprocess.Popen()

According to this SO answer, using subprocess.Popen() creates a new process with the same memory footprint as the Python code calling it. I noticed after a couple of days (sometimes a few hours!) of running AutoWX2 continuously that I was getting OSError: [Errno 12] Cannot allocate memory when running any bash script. That same SO answer recommends creating a single instance of subprocess.Popen() at the beginning of your Python script, and calling it with the appropriate command.

I created a new file called shell_scripts.sh which will run the command you give it until it completes. This bash script is called at the beginning of autowx2_function.py only once, so that only a small memory footprint is used. I have not seen memory issues since implementing this.

Note: this is probably screaming "memory leaks" to the average developer. I have not investigated this, so the underlying memory leakage problem might still be there.

Other changes

  • [NOAA module] Added options for map outlines (enable/disable) and image extensions (PNG or JPG).
  • autowx2version now uses the branch name, as git describe --tags gives me No names found, cannot describe anything.

I am accessing my RPi's terminal through SSH, and I have byobu installed to access multiple terminals through one SSH channel. I also have RPi-Monitor running to check memory consumption, CPU usage/temperature, and other information as the RPi runs.

I have not tested this PR on desktop machines, so I have no idea what will happen on them. I can only hope good things!

I'm not a backend/Python/Bash/Linux/JavaScript guru, so some of the code may look messy and might not conform to "standards". All the changes I made were to improve the performance on my Raspberry Pi and fix bugs that I had noticed.

Changes include:
- Debug messages for easier debugging.
- Implementation of NodeJS server with API code and Mustachio-style tags.
- Implemented missing installation packages.
- Include raw image from satellite to the gallery.
- Fixed "cannot allocate memory" error when using `subprocess.Popen`.
Changes:
- Created a function to mark printed messages as "debug". These are messaged that are useful for development and understanding what the code is doing, whilst also potentially displaying necessary values. The messages are *not* sent to the log file.
- Added ability to call pipe commands in the process. It beats having mutliple subprocesses running through Python, and just let the terminal/OS handle it.
- Modified the RTL Test method to look for a different string when checking for dongles. This is caused by the new method of calling "subprocesses" and the use of `printf` in the `rtl_test` source code.
- Added descriptive comments.
Changes:
- Added debug echos to show what is happening on the console.
- Fixed bug that was causing "RAW" to be passed as an actual enhancement (it's not).
- Removed duplicated code.
- Saving images as PNG instead of JPG.
Changes:
- Created a function to easily toggle debug messages.
- Implemented debugEcho function in noaa.sh and noaa_record.sh.
Also fixed issue where the image was being modified as JPG rather than PNG.
Image extension defaults to JPG, as it consumes much less memory. However, the quality is reduced. It is up to the user to decide which they prefer.
This is worse than the Fast and Furious franchise.
At this point, I have no idea what I'm doing, and I'm trying all possible alternatives.
@GlassesToCSharp
Copy link
Author

@filipsPL I give up. I don't have sufficient knowledge to understand how to fix the last Codacy issue. Can I ask for your help on this please?

@filipsPL
Copy link
Owner

filipsPL commented Feb 2, 2020

@ThomasBartleet thanks for the great effort you put into developement of autowx2! 👍 Good job! I didn't have time to look at it before, but seems to lookk very good. My only doubt is if these changes will fit only Raspberry Pi or the code will work on any system? This must be checked before the final merge.

About codacy - this one (popen) is indeed a pain in the ... neck. If this is the only issue, I guess we may have it there.

Maybe we should consider creating another pi-oriented branch for autowx2, and after we make sure the version works fine for all systems, merge it with the main branch?

@GlassesToCSharp
Copy link
Author

@filipsPL I think the changes should benefit any system, not just the RPi, but I do agree that this PR should be tested on other systems. I did try to keep it all as generic as possible! The only bigger change is the implementation of the Node.js server, but there seem to be tutorials on how to set up Node.js on multiple systems (Windows, Mac, Ubuntu, Beaglebone etc), so I don't think that should be a problem.

On another note, is there a setting in Codacy that you can tell it to ignore the Popen() warning/error?

Regarding creating a RPi-orientated branch, I think the only only differences between using AutoWX2 on an RPi and any other system is the installation. Otherwise, everything else is pretty much the same. Up to you! :)

@filipsPL
Copy link
Owner

filipsPL commented Feb 6, 2020

@ThomasBartleet thanks! I will check the code then and try to run it on my ubuntu. Also look at the codacy to ignore Popen().

Maybe other could also check this version on their system?

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

Successfully merging this pull request may close these issues.

2 participants