mgaman / PDUlib

Encode/Decode PDU strings for use with most GSM modems. Both 7 bit and 16 bit alphabets are supported.
GNU Lesser General Public License v2.1
22 stars 10 forks source link

Not an issue, but a question #18

Closed EliasA97 closed 2 years ago

EliasA97 commented 2 years ago

This library works fine. I have a question though. I recently found that on an arduino mega, encoding a simple 50-character sms comsumes about 700 bytes of ram. This is not very good for big projects. The thing is, does this library consume heap ram or stack? If it's stack I guess it's fine, but if it uses the heap it's going to be problematic if the board has to work long time.

mgaman commented 2 years ago

If you're talking about 50 characters in the GSM7 character set then this PDU library is not for you. Stick to TEXT mode and life is a lot simpler and cheaper in terms of RAM. For others who have even a single non-GSM7 character in a message then PDU is the only way to go. Looking at the private data section of the PDU object there are several static buffers allowing for encoding/decoding a message and its parts and they assume the worst case of maximum size. I suppose I could look at adding a defined maximum message size, both incoming and outgoing, and tailoring buffer sizes accordingly. I could also look at which buffers are never used together e.g. when encoding a message I may not need the buffer for decoding. A dual use buffer might save some space. Finally, do you use the PM compile time macro? That places the GSM7 translation tables in PROGMEM instead of RAM.

EliasA97 commented 2 years ago

I'm the one that told you about the greek alphabet problem, so I need to use PDU for my messages. What you're proposing is good, but I don't want to make you do any changes that are irrelevant to the general public. I can't understand what "PM compile time macro" is. This could probably free some ram. Would you explain a little more?

mgaman commented 2 years ago

Its all explained in the Development & Debugging section of the README of this github. If you don't want to do as described there, just edit the pdulib.h file and add the line

define PM

at the beginning. BTW what IDE are you using? When compiling for Arduino you also have to define the macro ARDUINO_BASE. Are you using the sources installed by Arduino IDE/PlatformIO ?

EliasA97 commented 2 years ago

Oh, I'll check the readme. I'm using the latest Arduino IDE (1.8.19).

EliasA97 commented 2 years ago

image

i did as you suggested but now I'm getting these compilation errors: In file included from C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:32:0: C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src/pdulib.h:243:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src/pdulib.h:249:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src/pdulib.h:255:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src/pdulib.h:263:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src/pdulib.h:272:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:40:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp: In member function 'int PDU::convert_utf8_to_gsm7bit(const char*, char*)': C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:159:32: error: 'lookup_UnicodeToGreek7' was not declared in this scope *a7bit++ = pgm_read_word(lookup_UnicodeToGreek7 + target - GREEK_UCS_MINIMUM); ^~~~~~~~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:159:18: error: 'pgm_read_word' was not declared in this scope *a7bit++ = pgm_read_word(lookup_UnicodeToGreek7 + target - GREEK_UCS_MINIMUM); ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:168:31: error: 'lookup_ascii8to7' was not declared in this scope short x = pgm_read_word(lookup_ascii8to7 + target); ^~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:168:17: error: 'pgm_read_word' was not declared in this scope short x = pgm_read_word(lookup_ascii8to7 + target); ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp: In member function 'int PDU::convert_7bit_to_ascii(unsigned char*, int, char*)': C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:352:26: error: 'lookup_ascii7to8' was not declared in this scope if ((pgm_read_byte(lookup_ascii7to8 + a7bit[r]) != 27)) { ^~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:352:12: error: 'pgm_read_byte' was not declared in this scope if ((pgm_read_byte(lookup_ascii7to8 + a7bit[r]) != 27)) { ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:368:42: error: 'lookup_Greek7ToUnicode' was not declared in this scope unsigned short S = pgm_read_word(lookup_Greek7ToUnicode + (a7bit[r]-16)); ^~~~~~~~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:368:28: error: 'pgm_read_word' was not declared in this scope unsigned short S = pgm_read_word(lookup_Greek7ToUnicode + (a7bit[r]-16)); ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp: In member function 'bool PDU::isGSM7(short unsigned int*)': C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:859:36: error: 'gsm7_legal' was not declared in this scope for (unsigned int i=0; i< sizeof(gsm7_legal)/sizeof(sRange);i++) { ^~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:861:18: error: 'pgm_read_word' was not declared in this scope if (*pucs >= pgm_read_word(&gsm7_legal[i].min) && *pucs <= pgm_read_word(&gsm7_legal[i].max)) ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp: At global scope: C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:894:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:1175:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:1326:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\PDUlib\src\pdulib.cpp:1347:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~

mgaman commented 2 years ago

What platform are you compiling for? I've only worked on AVR so have no idea if this works for other Arduino boards

EliasA97 commented 2 years ago

I'm using windows 11, arduino ide 1.8.19 as I mentioned above and my code is for arduino mega that has the ATmega 2560.

mgaman commented 2 years ago

You are using an old version, please update to 0.5.4 and try again

EliasA97 commented 2 years ago

I updated the library to 0.5.4, added the "#define PM" in the pdulib.h but now I get these errors:

In file included from C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:37:0: C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src/pdulib.h:258:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src/pdulib.h:264:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src/pdulib.h:270:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src/pdulib.h:278:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src/pdulib.h:287:7: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:45:5: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp: In member function 'int PDU::convert_utf8_to_gsm7bit(const char*, char*, int)': C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:172:34: error: 'lookup_UnicodeToGreek7' was not declared in this scope *gsm7bit++ = pgm_read_word(lookup_UnicodeToGreek7 + target - GREEK_UCS_MINIMUM); ^~~~~~~~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:172:20: error: 'pgm_read_word' was not declared in this scope *gsm7bit++ = pgm_read_word(lookup_UnicodeToGreek7 + target - GREEK_UCS_MINIMUM); ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:181:31: error: 'lookup_ascii8to7' was not declared in this scope short x = pgm_read_word(lookup_ascii8to7 + target); ^~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:181:17: error: 'pgm_read_word' was not declared in this scope short x = pgm_read_word(lookup_ascii8to7 + target); ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp: In member function 'int PDU::convert_7bit_to_unicode(unsigned char*, int, char*)': C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:445:24: error: 'lookup_gsm7toUnicode' was not declared in this scope if ((pgm_read_byte(lookup_gsm7toUnicode + gsm7bit[r]) != 27)) ^~~~~~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:445:24: note: suggested alternative: 'pduGsm7_to_unicode' if ((pgm_read_byte(lookup_gsm7toUnicode + gsm7bit[r]) != 27)) ^~~~~~~~~~~~~~~~~~~~ pduGsm7_to_unicode C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:445:10: error: 'pgm_read_byte' was not declared in this scope if ((pgm_read_byte(lookup_gsm7toUnicode + gsm7bit[r]) != 27)) ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:464:42: error: 'lookup_Greek7ToUnicode' was not declared in this scope unsigned short S = pgm_read_word(lookup_Greek7ToUnicode + (gsm7bit[r] - 16)); ^~~~~~~~~~~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:464:28: error: 'pgm_read_word' was not declared in this scope unsigned short S = pgm_read_word(lookup_Greek7ToUnicode + (gsm7bit[r] - 16)); ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp: In member function 'bool PDU::isGSM7(short unsigned int*)': C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:1053:39: error: 'gsm7_legal' was not declared in this scope for (unsigned int i = 0; i < sizeof(gsm7_legal) / sizeof(sRange); i++) ^~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:1056:18: error: 'pgm_read_word' was not declared in this scope if (*pucs >= pgm_read_word(&gsm7_legal[i].min) && *pucs <= pgm_read_word(&gsm7_legal[i].max)) ^~~~~~~~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp: At global scope: C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:1094:5: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:1375:5: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:1526:5: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~ C:\Users\anast\OneDrive\�������\Arduino\libraries\pdulib\src\pdulib.cpp:1547:5: error: 'PROGMEM' does not name a type PROGMEM ^~~~~~~

EliasA97 commented 2 years ago

Editing the pdulib.h file doesn't work for me and keeps giving errors. I'll try the instructions in readme and if I succeed I'll update my post for future reference.

mgaman commented 2 years ago

You have not defined ARDUINO_BASE. Follow the instruction of the README , Development & Debugging I found platform.txt in Win10 in \Program Files(X86)\Arduino\Hardware\arduino\avr

EliasA97 commented 2 years ago

Well, I tried the pdulib.h with both PM and ARDUINO BASE definitions and I got the same errors. I then tried changing the platform.txt. If I don't modify the pdulib.h but only the platform.txt, ram usage doesn't seem to change when I compile. Before I update the library I was using 59% of ram, after I got the latest build, the ram usage got at 60%. Then if I change the platform.txt I still get 60% of ram usage. I tried to edit both pdulib.h and platform.txt but I still get the same errors. By now I don't know what else to do, but I'll try anyother way to optimise my code. Also this is a photo of the platform.txt and the change I made like the readme says.

image

EliasA97 commented 2 years ago

After some research about progmem and by looking your code I concluded I had to do a simple thing that definitelly worked!!! More specifically:

In the pdulib.cpp everytime there is this part: image next is followed by a char array.

The trick is that you make these parts of the code: image into this: image

Also you have to put #include <avr/pgmspace.h> at the beggining of pdulib.h With that modification I get almost 0% ram consumption by the library.

mgaman commented 2 years ago

No no no, bad idea. You are forcing PROGMEM if needed are not. I want to get to the bottom of this problem. Please do as follows. Remove any mods you made to pdulib.h Change platform.txt as instructed in the README In Arduino IDE, go to file->preferences. Next to Show verbose output, click on Compilation. Restart IDE. Compile your pdu app. Click the lower pane of the IDE where the compilation output is, do Ctrl-A Ctrl-C to capture all the output. Open notepad, Ctrl-V to paste and save as a file. Send me the file.

Thanks, David

On Fri, Jul 1, 2022 at 12:24 AM EliasA97 @.***> wrote:

After some search about progmem and by looking your code I concluded I had to do a simple thing that definitelly worked!!! More specifically:

In the pdulib.cpp everytime there is this part: [image: image] https://user-images.githubusercontent.com/56277356/176780290-cb956a37-d855-4ab7-b121-dc85187402e2.png next is followed by a char array.

The trick is that you make these parts of the code: [image: image] https://user-images.githubusercontent.com/56277356/176780449-c728a123-4504-4861-be04-6528a29067fc.png into this: [image: image] https://user-images.githubusercontent.com/56277356/176780474-a02307ea-6742-4bac-b220-ffdd947ff3ee.png

With that modification I get almost 0% ram consumption by the library.

— Reply to this email directly, view it on GitHub https://github.com/mgaman/PDUlib/issues/18#issuecomment-1171692973, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3KSTX7FHW2YWLJMFYOQRLVRYF77ANCNFSM52EHQOPQ . You are receiving this because you commented.Message ID: @.***>

EliasA97 commented 2 years ago

I did 3 different compilations of the encode demo code. 1) With no modifications at all. Just the basic library. 2) With my proposal. 3) With your proposal.

1) using library with not any changes.txt 2) Using my version.txt 3) using your suggestion.txt

mgaman commented 2 years ago

OK, I do not see that ARDUINO_BASE has been defined. Here is an example compilation line from my machine

/home/henryd/.arduino15/packages/arduino/tools/avr-gcc/7.3.0-atmel3.6.1-arduino7/bin/avr-g++ -c -g -Os -w -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics -Wno-error=narrowing -flto -w -x c++ -E -CC -mmcu=atmega2560 -DF_CPU=16000000L -DARDUINO=10815 -DARDUINO_AVR_MEGA2560 -DARDUINO_ARCH_AVR -DARDUINO_BASE -I/home/henryd/.arduino15/packages/arduino/hardware/avr/1.8.5/cores/arduino -I/home/henryd/.arduino15/packages/arduino/hardware/avr/1.8.5/variants/mega /tmp/arduino_build_150769/sketch/Encode.ino.cpp -o /dev/null Alternatives for SoftwareSerial.h: @.***

Send me a copy of your platform.txt

On Sat, Jul 2, 2022 at 2:17 PM EliasA97 @.***> wrote:

I did 3 different compilations of the encode demo code.

  1. With no modifications at all. Just the basic library.
  2. With my proposal.
  3. With your proposal.

1) using library with not any changes.txt https://github.com/mgaman/PDUlib/files/9032909/1.using.library.with.not.any.changes.txt 2) Using my version.txt https://github.com/mgaman/PDUlib/files/9032912/2.Using.my.version.txt 3) using your suggestion.txt https://github.com/mgaman/PDUlib/files/9032913/3.using.your.suggestion.txt

— Reply to this email directly, view it on GitHub https://github.com/mgaman/PDUlib/issues/18#issuecomment-1172881464, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3KSTQHIDTQROE4RHOJSHDVSAQKXANCNFSM52EHQOPQ . You are receiving this because you commented.Message ID: @.***>

EliasA97 commented 2 years ago

First of all I want to thank you for still engaging resolving my small problem!

My platform.txt is from here: C:\Program Files (x86)\Arduino\hardware\arduino\avr

platform.txt

mgaman commented 2 years ago

Now I'm confused. I see in platform.txt the line build.extra_flags=-DARDUINO_BASE -DPM But it is not reflected in the compilation output you sent me From the same directory as platform.txt there is a file boards.txt, please send it to me

On Sun, Jul 3, 2022 at 11:52 AM EliasA97 @.***> wrote:

First of all I want to thank you for still engaging resolving my small problem!

My platform.txt is from here: C:\Program Files (x86)\Arduino\hardware\arduino\avr

platform.txt https://github.com/mgaman/PDUlib/files/9034380/platform.txt

— Reply to this email directly, view it on GitHub https://github.com/mgaman/PDUlib/issues/18#issuecomment-1173040589, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3KSTTCOQLWPJOV4EQIER3VSFIDFANCNFSM52EHQOPQ . You are receiving this because you commented.Message ID: @.***>

EliasA97 commented 2 years ago

Indeed it's a bit weird. I send you the boards.txt and on the meantime, I'll try to reinstall a fresh copy of arduino ide. If I get it to work, I'll inform you.

boards.txt

mgaman commented 2 years ago

Weird. I can't really debug your problem from afar so I'm coming to the conclusion that setting compilation time flags is a bad idea. I'm going to have to come up with a better solution. Let me work on it. I'll get back to you.

On Sun, Jul 3, 2022 at 6:59 PM EliasA97 @.***> wrote:

Indeed it's a bit weird. I send you the boards.txt and on the meantime, I'll try to reinstall a fresh copy of arduino ide. If I get it to work, I'll inform you.

boards.txt https://github.com/mgaman/PDUlib/files/9035097/boards.txt

— Reply to this email directly, view it on GitHub https://github.com/mgaman/PDUlib/issues/18#issuecomment-1173126797, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3KSTUWCZYVSI2CDRLLZ33VSG2HVANCNFSM52EHQOPQ . You are receiving this because you commented.Message ID: @.***>

mgaman commented 2 years ago

Here are my proposed changes making Arduino, not Desktop, the default environment. Restore platform.txt to its original form Restore pdulib.h to its original form In pdulib.cpp change

ifdef ARDUINO_BASE

to

ifndef DESKTOP_PDU

in pdulib.h, add or not add, as you wish. Note this really only has meaning for AVR boards.

define PM

On Sun, Jul 3, 2022 at 7:15 PM David Henry @.***> wrote:

Weird. I can't really debug your problem from afar so I'm coming to the conclusion that setting compilation time flags is a bad idea. I'm going to have to come up with a better solution. Let me work on it. I'll get back to you.

On Sun, Jul 3, 2022 at 6:59 PM EliasA97 @.***> wrote:

Indeed it's a bit weird. I send you the boards.txt and on the meantime, I'll try to reinstall a fresh copy of arduino ide. If I get it to work, I'll inform you.

boards.txt https://github.com/mgaman/PDUlib/files/9035097/boards.txt

— Reply to this email directly, view it on GitHub https://github.com/mgaman/PDUlib/issues/18#issuecomment-1173126797, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3KSTUWCZYVSI2CDRLLZ33VSG2HVANCNFSM52EHQOPQ . You are receiving this because you commented.Message ID: @.***>

EliasA97 commented 2 years ago

Well that did the trick! Now with these interventions I can have similar effects like with the PROGMEM that I did earlier. For fact, your encoding demo, consumes 21% of arduino mega's ram. After the changes it consumes 13%. The #define PM by the way still doesn't work for me but it's ok.

In conclusion, I believe we can call this a fix for anyone wanting ram optimisation.

mgaman commented 2 years ago

Great, I'll make the changes, update the README and issue a new release

On Sun, Jul 3, 2022 at 10:48 PM EliasA97 @.***> wrote:

Well that did the trick! Now with these interventions I can have similar effects like with the PROGMEM that I did earlier. For fact, your encoding demo, consumes 21% of arduino mega's ram. After the changes it consumes 13%. The #define PM by the way still doesn't work for me but it's ok.

In conclusion, I believe we can call this a fix for anyone wanting ram optimisation.

— Reply to this email directly, view it on GitHub https://github.com/mgaman/PDUlib/issues/18#issuecomment-1173160657, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3KSTW2KGYHOURALQCWIZDVSHVAVANCNFSM52EHQOPQ . You are receiving this because you commented.Message ID: @.***>