Portability #35

darksotmoon · 2024-06-16T18:14:01Z

darksotmoon
Jun 16, 2024

I have several suggestions to improve portability.

Currently it seems that wrench is targeted for arduino and linux / windows using gcc with c++ source. There are some portability issues when compiling to something else, in my case using the arm 6 compiler using C.

strnlen is not a standard c function - not all compiler libraries include this function. I suggest substituting a local implementation over using strnlen_s which is not well supported either. eg

extern "C" size_t strnlen(const char * s, size_t len)
{
const char * found = (const char *)memchr(s, '\0', len);
return found ? (size_t)(found-s) : len;
}

memory.h, sys/stat.h, sys/types.h and fcntl.h are not standard c header files. If no file system is being used, then memory.h is only used for the prototypes for memcpy and memset. These are in string.h, so I would suggest replacing memory.h with string.h. The file system headers with a STR_FILE_OPERATIONS wrapper or the WRENCH_xxxxx_FILE_IO defines.

Perhaps a config file would be useful. wrench.h to #include "wrench_config.h" and then set to default any settings which wrench_config did not define.

jingoro2112 · 2024-06-16T18:54:56Z

jingoro2112
Jun 16, 2024
Maintainer

Sounds great! I would love to increase portability. Any way I could get my hands on the tool stack you are using? maybe a docker image or something? I only need the compiler for validation of any changes made for the sake of portability like your strnlen() suggestion (which I plan to use)

…

On Sun, Jun 16, 2024 at 2:14 PM darksotmoon ***@***.***> wrote: I have several suggestions to improve portability. Currently it seems that wrench is targeted for arduino and linux / windows using gcc with c++ source. There are some portability issues when compiling to something else, in my case using the arm 6 compiler using C. strnlen is not a standard c function - not all compiler libraries include this function. I suggest substituting a local implementation over using strnlen_s which is not well supported. eg extern "C" size_t strnlen(const char * s, size_t len) { const char * found = (const char *)memchr(s, '\0', len); return found ? (size_t)(found-s) : len; } memory.h, sys/stat.h, sys/types.h and fcntl.h which are not standard c header files. If no file system is being used, then memory.h is only used for the prototypes for memcpy and memset. These are in string.h, so I would suggest replacing memory.h with string.h. The file system headers with a STR_FILE_OPERATIONS wrapper or the WRENCH_xxxxx_FILE_IO defines. Perhaps a config file would be useful. wrench.h to #include "wrench_config.h" and then set to default any settings which wrench_config did not define. — Reply to this email directly, view it on GitHub <#35>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIKAYWRB5JFQHECC77FM3ZHXIX3AVCNFSM6AAAAABJMZJDZKVHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZWHAZDOMJTHE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

0 replies

jingoro2112 · 2024-06-16T20:06:13Z

jingoro2112
Jun 16, 2024
Maintainer

quick audit shows I had been awful sloppy, I have removed all includes from the main project except for: #include <stdint.h> #include <stddef.h> #include <string.h> :)

…

On Sun, Jun 16, 2024 at 2:54 PM Curt Hartung ***@***.***> wrote: Sounds great! I would love to increase portability. Any way I could get my hands on the tool stack you are using? maybe a docker image or something? I only need the compiler for validation of any changes made for the sake of portability like your strnlen() suggestion (which I plan to use) On Sun, Jun 16, 2024 at 2:14 PM darksotmoon ***@***.***> wrote: > I have several suggestions to improve portability. > > Currently it seems that wrench is targeted for arduino and linux / > windows using gcc with c++ source. There are some portability issues when > compiling to something else, in my case using the arm 6 compiler using C. > > strnlen is not a standard c function - not all compiler libraries include > this function. I suggest substituting a local implementation over using > strnlen_s which is not well supported. eg > > extern "C" size_t strnlen(const char * s, size_t len) > { > const char * found = (const char *)memchr(s, '\0', len); > return found ? (size_t)(found-s) : len; > } > > memory.h, sys/stat.h, sys/types.h and fcntl.h which are not standard c > header files. If no file system is being used, then memory.h is only used > for the prototypes for memcpy and memset. These are in string.h, so I would > suggest replacing memory.h with string.h. The file system headers with a > STR_FILE_OPERATIONS wrapper or the WRENCH_xxxxx_FILE_IO defines. > > Perhaps a config file would be useful. wrench.h to #include > "wrench_config.h" and then set to default any settings which wrench_config > did not define. > > — > Reply to this email directly, view it on GitHub > <#35>, or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AALIKAYWRB5JFQHECC77FM3ZHXIX3AVCNFSM6AAAAABJMZJDZKVHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZWHAZDOMJTHE> > . > You are receiving this because you are subscribed to this thread.Message > ID: ***@***.***> >

2 replies

darksotmoon Jun 16, 2024
Author

I think also you might need stdbool.h

The Arm compiler can be downloaded ... if you jump through some hoops.

https://developer.arm.com/downloads/view/ACOMPE

darksotmoon Jun 16, 2024
Author

Also the blank code needs to be updated since it incorporates the version number.

wrench/discrete_src/debug/wrench_client_debug.cpp

Lines 30 to 33 in aee8f5c

    
           const unsigned char wr_blankCode[]= 
        
           { 
        
           	0x00, 0x00, 0x01, 0x02, 0xEE, 0x13, 0x25, 0xE1, 0xA4, 0x5A, // 10 
        
           };

0x00, 0x00, 0x01, 0x02, 0xEF, 0x14, 0xEF, 0x82, 0xA7, 0x99,
Or it could create it dynamically.

jingoro2112 · 2024-06-16T22:45:20Z

jingoro2112
Jun 16, 2024
Maintainer

hehe already did that, check the currently uploaded code :) The debugger is a work in progress, I bit off a lot and it wasn't clear anyone needed it, it's actually my next big project.

…

On Sun, Jun 16, 2024 at 5:17 PM darksotmoon ***@***.***> wrote: Also the blank code needs to be updated since it incorporates the version number. https://github.com/jingoro2112/wrench/blob/aee8f5c27d60c690f5d25e672b1c4c05794a8096/discrete_src/debug/wrench_client_debug.cpp#L30-L33 0x00, 0x00, 0x01, 0x02, 0xEF, 0x14, 0xEF, 0x82, 0xA7, 0x99, Or it could create it dynamically. — Reply to this email directly, view it on GitHub <#35 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIKA3GRX22AOEXN7T7D7LZHX6GTAVCNFSM6AAAAABJMZJDZKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TOOBYGUZTS> . You are receiving this because you commented.Message ID: ***@***.***>

0 replies

jingoro2112 · 2024-06-16T22:47:00Z

jingoro2112
Jun 16, 2024
Maintainer

(I haven't actually delete it yes, it just got swept up in the compat changes, the init() function compiles a blank string on the fly)

…

On Sun, Jun 16, 2024 at 4:12 PM darksotmoon ***@***.***> wrote: I think also you might need stdbool.h The Arm compiler can be downloaded ... if you jump through some hoops. https://developer.arm.com/downloads/view/ACOMPE — Reply to this email directly, view it on GitHub <#35 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIKA6EMB3NQZIV2PAJJS3ZHXWSZAVCNFSM6AAAAABJMZJDZKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TOOBYGM2DK> . You are receiving this because you commented.Message ID: ***@***.***>

0 replies

jingoro2112 · 2024-06-17T03:19:47Z

jingoro2112
Jun 17, 2024
Maintainer

Yeah I downloaded it but it wants me to pay for a licence to even compile code? sorry. I'll integrate any suggestions you have though.

…

On Sun, Jun 16, 2024 at 6:46 PM Curt Hartung ***@***.***> wrote: (I haven't actually delete it yes, it just got swept up in the compat changes, the init() function compiles a blank string on the fly) On Sun, Jun 16, 2024 at 4:12 PM darksotmoon ***@***.***> wrote: > I think also you might need stdbool.h > > The Arm compiler can be downloaded ... if you jump through some hoops. > > https://developer.arm.com/downloads/view/ACOMPE > > — > Reply to this email directly, view it on GitHub > <#35 (reply in thread)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AALIKA6EMB3NQZIV2PAJJS3ZHXWSZAVCNFSM6AAAAABJMZJDZKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TOOBYGM2DK> > . > You are receiving this because you commented.Message ID: > ***@***.***> >

0 replies

darksotmoon · 2024-06-17T15:43:35Z

darksotmoon
Jun 17, 2024
Author

My apologies - I did not realize that the standalone arm compiler was licensed. It's odd because the compiler can only be used for their products, so I don't see the benefit of restricting the use of it. I guess that also means that not that many non-professional uses are going to use the arm compiler.

With compiler 5 warnings enabled, which is more or less the norm, there are only a few problems.

"comparison of constant 16 with expression of type 'WRValueType' is always false"

wrench/discrete_src/cc/expression.cpp

Line 62 in aee8f5c

if ( value.type == WR_COMPILER_LITERAL_STRING )

wrench/discrete_src/cc/cc.cpp

Line 765 in aee8f5c

else if ( value.type == WR_COMPILER_LITERAL_STRING )

wrench/discrete_src/cc/cc.cpp

Line 2491 in aee8f5c

    
           uint32_t hash = (value.type == WR_COMPILER_LITERAL_STRING) ? wr_hashStr(token) : value.getHash();

I thought it might be because typed enums only were introduced in c++11 and the __cplusplus test is for C++98, but changing it to an untyped enum makes no difference. Probably something to do with anonymous structs on my compiler.
if ( (uint8_t)value.type == WR_COMPILER_LITERAL_STRING )
Casting removes the warning.

The compiler also does not like the little endian conversion marco, which assigned the variable to itself.
#define wr_x32(P) P
Addition of brackets removes that warning
#define wr_x32(P) (P)

The bottom line is that wrench compiles using the arm compiler 6 and appears to work.

I am making progress using the debug interface between a client and server the same PC, but not using the direct connection to the local server, but using a std::queue. Multiple threads are always a pain to debug.

0 replies

darksotmoon · 2024-06-17T16:01:59Z

darksotmoon
Jun 17, 2024
Author

Probably I should start to posts these as issues...

I believe the pre increment iterator should return a reference.

wrench/discrete_src/utils/simple_ll.h

Line 299 in aee8f5c

    
           const Iterator operator++() { if ( m_current ) m_current = m_current->next; return *this; }

Iterator& operator++() { if ( m_current ) m_current = m_current->next; return *this; }

And may as well add a post increment as well

Iterator operator++(int) { Iterator ret = *this; this->operator++(); return ret; }

0 replies

jingoro2112 · 2024-06-17T17:27:16Z

jingoro2112
Jun 17, 2024
Maintainer

I was surprised as well, why paywall the compiler? Made all changes as suggested, and also fixed up the debugger code so it actually doesn't crash and can step through a program, but the serial link is dicey and it is not great at showing symbols yet. That's a work in progress. As this library gets used let me know if there are shortcuts or fixes you would like to see, I'm very responsive developing it but also it's just me :)

…

-Curt

On Mon, Jun 17, 2024 at 12:02 PM darksotmoon ***@***.***> wrote: Probably I should start to posts these as issues... I believe the pre increment iterator should return a reference. https://github.com/jingoro2112/wrench/blob/aee8f5c27d60c690f5d25e672b1c4c05794a8096/discrete_src/utils/simple_ll.h#L299 Iterator& operator++() { if ( m_current ) m_current = m_current->next; return *this; } And may as well add a post increment as well Iterator operator++(int) { Iterator ret = *this; this->operator++(); return ret; } — Reply to this email directly, view it on GitHub <#35 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIKA5KYYIQQRC7QGOSSSTZH4CAZAVCNFSM6AAAAABJMZJDZKVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM4TOOJWGQ4DQ> . You are receiving this because you commented.Message ID: ***@***.***>

0 replies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Portability #35

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Replies: 8 comments 2 replies

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

Portability #35

darksotmoon Jun 16, 2024

Replies: 8 comments · 2 replies

jingoro2112 Jun 16, 2024 Maintainer

jingoro2112 Jun 16, 2024 Maintainer

darksotmoon Jun 16, 2024 Author

darksotmoon Jun 16, 2024 Author

jingoro2112 Jun 16, 2024 Maintainer

jingoro2112 Jun 16, 2024 Maintainer

jingoro2112 Jun 17, 2024 Maintainer

darksotmoon Jun 17, 2024 Author

darksotmoon Jun 17, 2024 Author

jingoro2112 Jun 17, 2024 Maintainer

darksotmoon
Jun 16, 2024

Replies: 8 comments 2 replies

jingoro2112
Jun 16, 2024
Maintainer

jingoro2112
Jun 16, 2024
Maintainer

darksotmoon Jun 16, 2024
Author

darksotmoon Jun 16, 2024
Author

jingoro2112
Jun 16, 2024
Maintainer

jingoro2112
Jun 16, 2024
Maintainer

jingoro2112
Jun 17, 2024
Maintainer

darksotmoon
Jun 17, 2024
Author

darksotmoon
Jun 17, 2024
Author

jingoro2112
Jun 17, 2024
Maintainer