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

Low memory available, stability problems may occur #19

Open
2 tasks
ElectricRCAircraftGuy opened this issue Apr 12, 2020 · 20 comments
Open
2 tasks

Low memory available, stability problems may occur #19

ElectricRCAircraftGuy opened this issue Apr 12, 2020 · 20 comments
Labels
help wanted Extra attention is needed

Comments

@ElectricRCAircraftGuy
Copy link
Collaborator

ElectricRCAircraftGuy commented Apr 12, 2020

When I build the project for the Arduino Nano I see the following output in the Arduino IDE:

Sketch uses 18280 bytes (59%) of program storage space. Maximum is 30720 bytes.
Global variables use 1882 bytes (91%) of dynamic memory, leaving 166 bytes for local variables. Maximum is 2048 bytes.
Low memory available, stability problems may occur.

This is a problem, as it means the stack is nearly completely used up by global variables, so local variables in any scope or code section could potentially overflow the stack really easily. I think I can do some code cleanup to fix this, perhaps by moving strings to progmem with the F() macro, or using other coding techniques. I'll have a look at the code.

Read more on the F() macro here:

  1. https://www.arduino.cc/reference/tr/language/variables/utilities/progmem/
  2. https://www.baldengineer.com/arduino-f-macro.html

UPDATE 20 APR 2020:

The bulk of this issue is resolved now that the two big lookup tables in the code are in progmem, but let's still not close this issue until at least all strings have been surrounded by the F() macro and all large const arrays or structs (if any) have been moved to progmem.

  • all string literals are inside an F() macro
  • all large arrays or large data types (> ~16 bytes or so) are in PROGMEM)
@jsodergren
Copy link

jsodergren commented Apr 12, 2020

For me, that warning went away with commit ae4205c.. on Apr 6.
Building the Apr 6 version for a Nano:

Sketch uses 17810 bytes (57%) of program storage space. Maximum is 30720 bytes.
Global variables use 1518 bytes (74%) of dynamic memory, leaving 530 bytes for local variables. Maximum is 2048 bytes.

But it looks like the Apr 8 reformatting/cleanup commit reverted the changes of the three previous commits.

Additionally, the F() macro is getting clobbered by redefining F for use as a PID parameter:
#define F 5 // motion control feed forward
I've changed the PID parameter name to "FF" to work around this.

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 12, 2020

Are you working with the team? Making one of these solo with their code? Something else?

But it looks like the Apr 8 reformatting/cleanup commit reverted the changes of the three previous commits.

That's important. That's too bad. Using git is a fairly complicated skill so it's easy to mess up. Can you give me a link to a branch that was prior to the merge and point out which commits were lost? I can look at them and see what was changed and put the changes back in if necessary.

Additionally, the F() macro is getting clobbered by redefining F for use as a PID parameter:
#define F 5 // motion control feed forward
I've changed the PID parameter name to "FF" to work around this.

Good to know. We need to fix this. I'm about to start cleaning up the code but my first 2 PRs already touch like 230 files or something, so until they merge I'm afraid to change too much more. I'll end up touching nearly every line by the time I'm done and I don't want to mess up work underneath them.

My 2 PRs waiting now are #18 and #20.

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 12, 2020

@jsodergren, If you're working on the code right now, watch out for my changes! If so, you should pull my #20 branch right now and rebase on top of it. Let me know if you're working on stuff so I can be aware.

@jsodergren
Copy link

jsodergren commented Apr 12, 2020

Good to know of your work and PRs. I haven't submitted any PRs (yet); just trying to modify the released code slightly to work with our build (I'm part of a small team in Alaska working to replicate the device). Aside from the already-mentioned macro issue, I'm using a different LCD panel (NHD-0420D3Z 20x4) and have written a minimal library to use it with this code.

From what I can tell, the Apr 8 commit (a480902) reversed the changes made in commits 352b36a, e8fb1d9, and ae4205c. Here's the point right before those changes were reversed; this is what I'm currently working with: https://github.com/AmboVent/AmboVent/tree/352b36a447adc2dabcf7890711da37fb903b46d6

@giorakor
Copy link
Collaborator

just released a new ver. that returned the lost functionality (accepted a PR that suggested readability improvements without noticing it was based on old ver.)
in this version -

  • improved the code structure, memory use (75% now.. still warns.. but it is much better)
  • added some new functions
    - press test once during standby to run one cycle
    - ability to calibrate the env. pressure from the maint menu
    - option to set the breath cycle movement time

@nimrod46
Copy link
Member

nimrod46 commented Apr 12, 2020

Hi, thank you guys for your input, @giorakor improved the code a lot from the last commit and fix the merge #14 that was based on an older version

@nimrod46 nimrod46 reopened this Apr 12, 2020
@nimrod46 nimrod46 added help wanted Extra attention is needed in work Feature or improvment that is still under develop and removed in work Feature or improvment that is still under develop labels Apr 12, 2020
@Arie001
Copy link

Arie001 commented Apr 12, 2020

I have just today submitted a new pcb . I plan on porting the code to esp32 all the Libs used seem to be supported. A simple next gen pcb will be esp32 no memory shortage and not running 24bit math on a 8 bit ATmel. This is my solution a hardware upgrade for me to replace the nano with a esp and Jtag can start if needed.

I can have that version done by the end but I only have one ESp32-wroom that I can debug with .
I have a Jtag cable so it is normal code development with breakpoints.

Sorry if it is a 10 pound hammer solution.

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 12, 2020

@Arie001, the real problem was coding. Simply using progmem and the F() macro could decrease RAM usage from 90% to 30%, for instance. I see they added just a couple usages of progmem and it's already down from 91% or so to like 55% RAM used by global variables.

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 13, 2020

Note: please don't close this issue until at least all strings have been surrounded by the F() macro and all large const arrays or structs (if any) have been moved to progmem.

@Xexnon
Copy link

Xexnon commented Apr 13, 2020

I have just today submitted a new pcb . I plan on porting the code to esp32 all the Libs used seem to be supported. A simple next gen pcb will be esp32 no memory shortage and not running 24bit math on a 8 bit ATmel. This is my solution a hardware upgrade for me to replace the nano with a esp and Jtag can start if needed.

I can have that version done by the end but I only have one ESp32-wroom that I can debug with .
I have a Jtag cable so it is normal code development with breakpoints.

Sorry if it is a 10 pound hammer solution.

Porting the code to esp32 you might experience issues with ADCs on esp32. The ADC values are non-linear. You get the same ADC resolution for high and low voltages. Then also the error between actual measurement and value measure by ADC is about 10%

@Arie001
Copy link

Arie001 commented Apr 13, 2020

I do not know where you get your esp32 or but a esp can read a2d Just Fine no where near 400 counts error If all it has to do is measure a a2d of a pot . Advise a rc filter cap on the a2d pin so are you saying .And No at 0db is not the same as -3db put into the same a2d

We are talking about memory 92% vs 5%

Platform IO
RAM: [= ] 5.2% (used 17076 bytes from 327680 bytes)
Flash: [== ] 18.8% (used 246093 bytes from 1310720 bytes)

i2C 4 Leds 3 switches 3 Pots and a analog feedback is not pushing the limit . It is alive on the uart at 115200

@Xexnon
Copy link

Xexnon commented Apr 13, 2020

I do not know where you get your esp32 or but a esp can read a2d Just Fine no where near 400 counts error If all it has to do is measure a a2d of a pot . Advise a rc filter cap on the a2d pin so are you saying .And No at 0db is not the same as -3db put into the same a2d

We are talking about memory 92% vs 5%

Platform IO
RAM: [= ] 5.2% (used 17076 bytes from 327680 bytes)
Flash: [== ] 18.8% (used 246093 bytes from 1310720 bytes)

i2C 4 Leds 3 switches 3 Pots and a analog feedback is not pushing the limit . It is alive on the uart at 115200

Wow.. Makes a lot of sense... Seems like you're done porting it to esp32

About the ESP ADC issue espressif/esp-idf#164 here you go

espressif/arduino-esp32#92

@Xexnon
Copy link

Xexnon commented Apr 13, 2020

I do not know where you get your esp32 or but a esp can read a2d Just Fine no where near 400 counts error If all it has to do is measure a a2d of a pot . Advise a rc filter cap on the a2d pin so are you saying .And No at 0db is not the same as -3db put into the same a2d

We are talking about memory 92% vs 5%

Platform IO
RAM: [= ] 5.2% (used 17076 bytes from 327680 bytes)
Flash: [== ] 18.8% (used 246093 bytes from 1310720 bytes)

i2C 4 Leds 3 switches 3 Pots and a analog feedback is not pushing the limit . It is alive on the uart at 115200

would you mind sharing the code and pcb design

@Arie001
Copy link

Arie001 commented Apr 14, 2020

off topic

The ESP32 has a native range of 1.1v Full so 1.1/4096 is scale if you measure on 3.3v full scale it is 3.3 /4096 -6db . To say the error is the same no matter if I use 1.1v or 3.3v shows a lack of Knowledge.Do you claim I put 500mv dc onto a a A2D I get either 450mv or 650mv no matter if I am on a 1.1v range ore 3.3V.The Error is the same no matter The Range.The a2d in not 100 Linear
I Know that.If you look people are able to measure 0.02v . 12 years back when I worked for Intel the Chips were not 100% Linear I debugged many a A2D problem on old LTX VLSI testers.
Been there done That .

On topic

my code is the same code as the this project imported to platform io I changed the the servo library to https://www.arduinolibraries.info/libraries/esp32-servo it did not compile it had issues with the variable called index char * to int conversion fail . I renamed Index Index_arie it compiled .download. I only have esp32 bare modules not a devkit .

For Idea of how I will be doing the circuit please see

https://arie-lashansky.info/corona/carona_HX711.pdf

My Style is to be very careful Esp top right

2esp pots page 1 bottom left dual pots below esp ( Backup Motor feed Back too)
2 voltage monitors ( no hx711 the pressure sensor is is not nxp but the )
2
te sensor one on 3v one 3v Aux one to main esp 1 to Aux
2 power supply may be the idea on page 2.
Display is a TFT 2.5 inch or 3.2 Inch spi touch display.

Who knows I may even look at a second full mechanical backup.second Ambo bag

As my Input is not wanted or needed here. I all allowing myself to diverge from the design done by the 40 experts engineering and medical experts .

From my circuit my style is clear no way in hell will I put a pull up to 4.7k pull up to 5v on a device that the datasheet says max IO voltage in VDD . I put two sensors in the box and box not 1 .

https://www.te.com/commerce/DocumentDelivery/DDEController?Action=showdoc&DocId=Data+Sheet%7FMS5803-01BA%7FB3%7Fpdf%7FEnglish%7FENG_DS_MS5803-01BA_B3.pdf%7FCAT-BLPS0038 ( page 5 max allowable voltage on pins SCL and SDA)

Note Jumpers allow cheap and 100% backup ( They are be wire links)

It is a Question of how you view your design . I am not a mechanical engineer and my software skills are limited .

Just a unemployed Electronics person that has over 30 years no second degree in Engineering not
doing a Public relations exercise for Israeli defense company of Magaen David Adom

Who I am is clear where I come from is clear I have even added my resume the one that
has not found work in over a year and note even my resume is clear I do not Lie and call myself
things I am not qualified like a Degree engineer.

Who I am is clear as daylight did I know a closed pcb was in the works for this design yes.
was if part of the Idea to fork the design yes most definitely. Part of open source design
nobody owns the IP.

Did I need need to generate my own DXF Yes never done before my me in a design where I was
unable to get DXF.
I my pcb missing a mounting hole in the bottom right corner Yes no hole in the metalwork.
Is my Switch to near to the mounting hole (Test ) I did not put switch in that position Ambovent did.
Have I uses witches with a 8mm head not 6.5mm yes No cheap option Alpms .

Mix This the two pcbs below remove the (nxp pressure sensors and Arduino). I will be a 4 layer board.3v 3vaux 5v and 5v_aux 12v 12aux on split planes.

https://grabcad.com/library/ambervent_pcb-baisic-placement-1
https://grabcad.com/library/pcb-exported-from-kicad-1
This is placement of the extender

‏‏arie_lashansky_cv.docx
carona_HX711.pdf

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 14, 2020

As my Input is not wanted or needed here. I all allowing myself to diverge from the design done by the 40 experts engineering and medical experts .

Hey we didn't say that. I'm just a random guy who started poking at this project a few days ago--not part of the team. All I know is, before I go changing something I have no control over (the entire microcontroller, for instance), I'd like to get the code cleaned up. My goal: clean the code. Make it robust. If I do that. I will be happy. Based on what I see, the Nano is more than adequate. I've been using it for thousands of hours and it's tough to actually hit its limits--they definitely aren't, from what I see skimming the code. So...back to cleaning the code and making it better. They did a great job, I just want to contribute what I can.

not doing a Public relations exercise for Israeli defense company of Magaen David Adom
Also, I don't even know what this means.

was if part of the Idea to fork the design yes most definitely. Part of open source design
nobody owns the IP.

I don't know what you're saying here either.

nobody owns the IP

For this project, their license seems to indicate this is true, as it releases it to public domain. For most open source projects (let's say...I don't know--99% of them), this is absolutely false. Someone owns the IP--they just provide open source licenses to anyone is all. I own all the IP on my open source projects, pure and simple. They are licensed--by me--with open source licenses.

We have diverged massively from the point of this thread to fix some memory issues.

And I hope you can find good work. Remember to use LinkedIn, network with everyone, and practice interviewing skills. Be willing to move anywhere required--wherever the jobs are. Also have a solid resume and showcase your skills with past and present projects--personal or otherwise.

@Xexnon
Copy link

Xexnon commented Apr 14, 2020

off topic

The ESP32 has a native range of 1.1v Full so 1.1/4096 is scale if you measure on 3.3v full scale it is 3.3 /4096 -6db . To say the error is the same no matter if I use 1.1v or 3.3v shows a lack of Knowledge.Do you claim I put 500mv dc onto a a A2D I get either 450mv or 650mv no matter if I am on a 1.1v range ore 3.3V.The Error is the same no matter The Range.The a2d in not 100 Linear
I Know that.If you

my code is the same code as the this project imported to platform io I changed the the servo library to https://www.arduinolibraries.info/libraries/esp32-servo it did not compile it had issues with the variable called index char * to int conversion fail . I renamed Index Index_arie it compiled .download. I only have esp32 bare modules not a devkit .

Had the same issue while porting to esp32 yesterday, the servo, and LCD libraries are incompatible..

Still had the issue with Index on line 138.. I think it should be renamed to something else on the main code for people who will want to port the code to other platforms

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 14, 2020

Please open a new issue and link to it here. I need some clarity. What variable where needs to be renamed to what, due to what error?

Here's line 138: https://github.com/AmboVent-1690-108/AmboVent/blob/master/3-Software/Arduino/ventilation_machine/ventilation_machine.ino#L138

int motorPWM,index=0, prev_index,i, wait_cycles,cycle_number, cycles_lost,index_last_motion;

@Arie001
Copy link

Arie001 commented Apr 14, 2020

I renamed the Index Issue as part of my Debug in getting it to compile I do it often to find where I have made changes. It is a way of me finding my Changes it is part of my workflow. I do not leave it that way.To search and replace Index_arie with Main_index is simple . It is part of debugging.

Index is in 24 places in the code that is about 750 lines I use that method for Debug why do you think I am not uploading the code . I do not even Know on what Lib Index was defined Char*

The Time was spent to see the memory Footprint no more no Less . I am not a Embedded Software
person. If I touch Code that I have not Read or tried to understand any Line I mark with my name saves me getting mixed up.

I am working in a environment called Platform IO on Visual Studio code on a Part ESP32 not related
to Arduino Nano Atmel. Why Open a Issue where the Issue was created by me Porting From Arduino Ide to Platform IO it is a issue for me nobody else, Like the person looking for

I needed to Change Sevo. to the ESP version the simple solution was the one I took it.
My design I have not even decided if I want to use a base esp like my Hx711 or a module or if
I want to make it hardware redundant like the HX711 why open Issues that do not exist

The Part from the Sevo32 Lib is not the problem but shows how hard it is to Find

esp32servo

index_arie

index_arie

@Arie001
Copy link

Arie001 commented Apr 14, 2020

This is all I wanted to see no more no less

goal_achieved

@Arie001
Copy link

Arie001 commented Apr 21, 2020

This issue can be closed current version only uses 1k so 1k Free

https://github.com/AmboVent-1690-108/AmboVent/tree/master/3-Software/Arduino/ventilation_machine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants