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

Python wrapper for the Wirepas C Mesh API library #23

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

Conversation

PFigs
Copy link
Contributor

@PFigs PFigs commented Jan 8, 2020

This pull request implements a Python wrapper for the Wirepas C Mesh API
library, done using the Cython compiler: https://cython.org. Tested
with Cython v0.29.14. Build script is very rudimentary at this point.

@PFigs PFigs added the enhancement New feature or request label Jan 8, 2020
@PFigs PFigs requested a review from GwendalRaoul January 8, 2020 06:55
For the best user experience, typing "nmake" in the Microsoft Visual Studio
Developer Command Prompt should build the Wirepas C Mesh API library
successfully. The syntax of NMAKE and GNU make makefiles are sufficiently
different that a single makefile will not work for both.

This commit adds automatic detection of the make tool in use, and calls the
suitable tool-dependent makefile. Working NMAKE makefiles will be added in
another commit.
This commit refactors the platform-specific code and splits it in POSIX,
Linux and Darwin (macOS) parts. Both Linux and Darwin use the POSIX
functionality, requiring just a small amount of OS-dependent code.

Build support with Clang etc. on macOS is also implemented. The build
platform is auto-detected by the GNU make makefile.
As outlined in [CONTRIBUTING.md](CONTRIBUTING.md), path variables in
makefiles should not have a trailing path separator. This commit fixes
that.
Turns out that optimization was not turned on for the C Mesh API library.
Added the compiler flags to turn on optimizations.
The MSVC compiler does not understand the GCC __attribute__ syntax, but uses a
__pragma keyword instead. There's a pragma that can be used to enable struct
packing for a section of code instead of per type in the type definition,
like GCC does.

To support both GCC and MSVC in the same source code, an extra header file
called "compiler_ext.h" is introduced in this commit. It contains the
necessary macro trickery for managing packed structs.
The internal APIs of the SLIP module had some type inconsistencies and some
comments were mangled. This commit cleans up the module and also adds some
error checking that was missing.

Also, variable length arrays (VLAs) were turned into fixed size arrays, because
MSVC does not support VLAs.
…ility

Microsoft Visual Studio C compiler does not seem to like the multi-line
macros in msap.c and wpc.c. The macros have now been converted to static
inline functions.
When a packet is sent, an optional TX callback is called. There was a bug
in converting the buffering delay from an internal representation to
milliseconds. The TX result (sent / discarded) was also wrong.

This commit also fixes some unrelated comment spelling and formatting errors.
This commit adds NMAKE makefiles and a dummy versions of platform-specific
functions for Windows. This allows compiling the Wirepas C Mesh API library
on Windows, in Visual Studio Developer Command Prompt, by calling NMAKE.

Actual implementation of Windows-specific functions will be added in
another commit.
Some pointer parameters didn't have the const qualifier even though the
parameters were read-only. This commit adds the missing const qualifiers
to the public and internal APIs.

Please note: this commit changes the public API. The changes are mostly
backward compatible, but some callback prototypes have changed. As a result,
const qualifiers have to be added to any user-defined callback functions, or
else the code won't compile.
Some log message formatting and wording was inconsistent, so it was improved.
Also, a couple of potential bugs related to logging were fixed, as well as
some typos in comments.
Some error situations were not properly handled, so a couple of check were
added.
Even if the library file already exists, run make in the lib/ directory.
This ensures that any changes to the library are compiled as well.
This commit adds serial port, thread and logging support for Windows.
Add NMAKE makefiles and some small fixes to compile cleanly.
In order to use the Wirepas C Mesh API library in a Python extension module,
it has to be built with flags that are compatible with the official Python
release for Windows. The default flag /MT ("link with LIBCMT.LIB") needs to
be changed to /MD ("link with MSVCRT.LIB").
Move Windows toolchain settings in a separate makefile fragment and
harmonize the messages that are printed during the build for both
GNU and Windows platforms.

def app_config_data_callback(app_config_data):
sys.stdout.write(
"\nIn app_config_data_callback(app_config_data = %s)\n" % repr(app_config_data)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)

@jy-wirepas jy-wirepas changed the title Preliminary Cython wrapping for the C-Mesh-API library Python wrapper for the Wirepas C Mesh API library Jan 18, 2020
@jy-wirepas jy-wirepas marked this pull request as ready for review January 18, 2020 07:33
This commit implements a Python wrapper for the Wirepas C Mesh API
library, done using the Cython compiler: https://cython.org. Tested
with Cython v0.29.14. Build script is very rudimentary at this point.
@PFigs
Copy link
Contributor Author

PFigs commented Jan 29, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 3
           

Complexity increasing per file
==============================
- python/testbench.py  2
- python/setup.py  2
         

Clones added
============
- lib/platform/win32/logger.c  1
- lib/platform/win32/platform.c  2
- lib/platform/posix/platform.c  2
- lib/platform/posix/logger.c  1
         

See the complete overview on Codacy

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

Successfully merging this pull request may close these issues.

2 participants