feat: add VOIP with permission verification to Kivy #823
Conversation
- Integrate VOIP functionality for Android developers - Implements permission verification for secure access - Allows for auto or manual selection of SSL/TLS protocols to secure VOIP call - Gracefully ends call automatically if the connection drops unexpectedly Full project example: https://github.com/SanquezH/Kivy-VOIP
|
This is my first time submitting a change, and I just wanted to check in on the status of this PR. If there’s anything I can do to help or adjust the changes, please let me know. |
Integrate VOIP functionality for developers Includes support for SSL/TLS to ensure encrypted communication Implement permission verification for microphone access Full project example: https://github.com/SanquezH/Kivy-VOIP
|
Looks like you are missing out to fill in for other OS's so it will throw a "not implemented" error. If you are able to implement this code with other OS's it would be appreciated if it did! |
|
I specialize in Android app development, which is why this code is currently designed for Android only. While other OSs aren’t supported at the moment, I’m open to future collaboration to implement iOS support. I’ve recently started exploring iOS development with Kivy after getting a MacBook two weeks ago, so I’m still catching up. |
Still remains. Have a look on how other implementations are doing. Take your time :) |
|
The temperature, humidity, and IR Blaster modules are currently supported only on Android. What steps can I take to ensure the same level of acceptance? |
|
In |
•Integrate VOIP functionality for developers •Includes support for SSL/TLS to ensure encrypted communication •Implement permission verification for microphone access •Full project example: https://github.com/SanquezH/Kivy-VOIP •Incorporates debug statements using Kivy's built-in logging system
|
@kuzeyron The platform check is removed in the latest commit change. |
|
@SanquezH My hope is that these comments are helpful to you in getting this PR accepted. I'm not a maintainer, I'm also not an expert at plyer - but I see a few things that need attention. The code does not seem to "match" the plyer framework. Lets use accelerometer as an example. The facade defines a parent class that provides a platform independent interface to an accelerator. The private and public interfaces are defined. You have added a new interface, enable_debug() this is unique to VOIP, and does not match the rest of the framework. I'd recommend removing this. You also have a number of class variables in the facade. Looking across the library, I do not see this done in the facade, instead the facade defines a property (@Property), and uses calls to access these kinds of attributes. See the implementation of acceleration in facades/accelerometer.py. Given I don't see this kind of use of class vars in any of the existing capabilities, it would seem most if not all of these attributes could be parameters to a make_call() method, matching the call implementation. The implementation of VOIP does not work with the facade. Again lets look at the call class as an an example. The android implementation of the Call class is names AndroidCall, and derived from the facades Call class. The VOIP implementation needs to match this structure so the facades can dynamically select the appropriate implementation for the target platform. Also note in android/call.py there is a function instance() that returns an instance of Android call. This is consistent in all of the platform files, and required for the proper implementation of the library. To better understand the Plyer library take a look at plyer/init.py You can see that the Proxy class is instanced for each "component" in the library. Looking at plyer/plyer/utils.py we can see the Proxy class implementation. This code is rather sophisticated, looking at the _ensure_obj() method we can see that instance() function is called here from the appropriate file. The getattribute method will call ensure_obj() when an attribute the create proxy class is made. Hope that helps - thanks for making this contribution! |
Look at the facade for irblaster, you will see the private methods raise a NotImplemented() Error. You will want to add this to the VOIP facade. When the AndroidVOIP is instanced, these will be overridden. For platforms that do not have an implementation - the exception will be raised. |
|
@ElliotGarbus Thank you for your feedback, I will have to wait until this weekend to make any changes to the facade file. |
Allows user to use dot notation or **kwargs to for _start_call() function. Removed excessive variables. Removed comments. Removed enable_debug() function and implemented its functions in __init__().
Incorporated client.enable_debug() functionality into __init__().
|
@ElliotGarbus & @kuzeyron I think the VOIP module is finally incorporated into the Plyer module as desired. What do you think? Facade: https://github.com/SanquezH/plyer/blob/VOIP/plyer/facades/voip.py |
Ready for review.
Ready for review.
Fixed example usage comment.
Ready for review.
Facade comment final update
Accounts for detecting when client disconnects from server.
|
@SanquezH Overall very well done. Here are a few things to consider that I expect the maintainer will have a strong opinion on. I don't know the direction for the library - so consider these food for thought. If the direction is not to have logging in the lib (as it appears now - you could make the self.debug a private option to get the logging messages. All of your log messages are debug. I wonder the messages that get turned on if the debug parameter is set should be info level messages. They seem more informational, and would not require the logging level to be set to debug to be captured. There are places where you are capturing exceptions, it would make sense to me to have these logged as exceptions - independent of the debug flag. |
|
@ElliotGarbus Thank you for helping me throughout the process, I learned a lot from this. I will change the log levels to info or error where applicable. In the change I submitted about 45 minutes ago, self.debug is a private variable. If the maintainers prefer the optional logs to be removed, I will gladly comply. However, for now, I would like to retain the logs to provide developers with an easy way to identify issues directly, without relying on external sources. |
|
@SanquezH You're welcome. Your choices make sense to me. Note there is a log message for exceptions. Good Luck! |
Removed all debug level logs and replaced them with their applicable level.
Debug level logs were removed to reflect appropriate log levels in plyer's VOIP module.
Allows developer to catch permission or connection error to display appropriate error dialog or message.
Allows developer to catch permission or connection error to display appropriate error dialog or message.
|
The latest updates to AndroidVoip and the VOIP example improve error handling, allowing developers to identify critical issues and present them to users through dialogs or messages in the following scenarios: I have tested the code, and it performed as expected. I will refrain from making any further changes unless directed by a maintainer. |
Full project example: https://github.com/SanquezH/Kivy-VOIP