smartdevicelink / sdl_ios Goto Github PK
View Code? Open in Web Editor NEWGet your app connected to the 🚙, make your users feel like a 🌟
Home Page: www.smartdevicelink.com
License: BSD 3-Clause "New" or "Revised" License
Get your app connected to the 🚙, make your users feel like a 🌟
Home Page: www.smartdevicelink.com
License: BSD 3-Clause "New" or "Revised" License
This is completely unnecessary, it's importing a ton of code here for no reason.
It's the only enum i've seen that doesn't use constants
currently defined as
+(NSString*) NEGATIVE_JINGLE { return @"NEGATIVE_JINGLE"; }
+(NSString*) POSITIVE_JINGLE { return @"POSITIVE_JINGLE"; }
+(NSString*) LISTEN_JINGLE { return @"LISTEN_JINGLE"; }
+(NSString*) INITIAL_JINGLE { return @"INITIAL_JINGLE"; }
+(NSString*) HELP_JINGLE { return @"HELP_JINGLE"; }
see #3 for more information
In the code, the raw values are strings: request
, response
, and notification
. However, in the spec, they are defined as 0
, 1
, and 2
respectively. PR #33 will cover this issue. A standard NS_ENUM
will be created for this case.
Since the project is open source, they can dig in to the meat of the library and compile it themselves if they need to. Hiding files that integrators don't need is a great way to help out developers who are getting started with the library review only what is necessary for them to use the library.
To do this, the compile settings for each "private" header file will be changed from public to either project or private (I don't currently remember what the difference is, but we did this with Livio Connect, so it would be easy to check). Then, you have to make sure that all the header files that will be public contain no imports of private header files, and only use the @class
tag. Private header files can be included in .m files.
Lots of enum objects permanently in memory now, could do conversion between String Enum <–> Integer Enum instead. Less memory overhead, probably more CPU overhead, likely negligible. Removes an enormous amount of code.
Right now, Enums are designed as objects. From what I understand, there are a few reasons for this:
See the comment below for more details.
Also, see #425, perhaps just typedef EnumName NSString
and stringly typed constants?
The initWithDictionary
function in the SDLRPCMessage
class assumes that whatever dictionary is passed in is in a specific format.
As a result, if a dictionary that doesn't follow this format is passed in, then the function will raise an exception which means nothing to the user.
For Example, this exception is thrown when a dictionary containing a single key:value pair is given to the function:
"failed: caught "NSInvalidArgumentException", "-[SDLAppInterfaceUnregisteredReason objectForKey:]: unrecognized selector sent to instance 0x7f9dc349e9e0""
The function should check whether the dictionary follows the correct format, and return nil or throw an exception describing the problem if it does not.
We should document all of our methods next to the methods in header files. This will provide us with in Xcode documentation generation for alt-clicking on methods; as well as the opportunity to generate html based on the documentation.
From Apple
Although Objective-C includes syntax for exception handling, Cocoa and Cocoa Touch use exceptions only for programming errors (such as out of bounds array access), which should be fixed before an app is shipped.
All other errors—including runtime problems such as running out of disk space or not being able to access a web service—are represented by instances of the NSError class. Your app should plan for errors and decide how best to handle them in order to present the best possible user experience when something goes wrong.
I've noticed a few places in the proxy where exceptions are used as flow control, those should be refactored.
Which means when comparing we're just comparing pointers. This works right now because we're using global objects, but it's bad practice, and resistant to testing and refactoring.
38 times it's spelled 'recieve'. To fix this will be backward incompatible, because it's in class names and file names.
See #21
Generally determinism is better. What's up with the randomness (between 0 and 0.5 sec)? Why not just implement a single retry delay?
//Begin Connection Retry
float randomNumber = (float)arc4random() / UINT_MAX; // between 0 and 1
float randomMinMax = 0.0f + (0.5f-0.0f)*randomNumber; // between Min (0.0) and Max (0.5)
Hi,
I assume that this issue tracker, here at GitHub, is the authoritative issue tracker for SDL. I say that because there are some lingering issues in GENIVI's bug tracker and I don't want them to get lost and lonely over there: http://bugs.genivi.org/buglist.cgi?query_format=specific&order=relevance%20desc&bug_status=__open__&product=SmartDeviceLink&list_id=2505
It likely wouldn't take much to move them here, if that's needed. Or, if they're resolved, they can get closed.
Thanks!
For instance -[SDLBitsPerSample _8_Bit]
should be -[SDLBitsPerSample 8_BIT]
The library is in a state at this point that it is inevitable that we will be changing public apis, as nearly every API is public. This is not ideal for the semantic versioning structure currently employed by this project.
However, there is a loophole. Point 4 of Semver states:
Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.
I realize that version 3.0.0 is where we started the project, but I believe the state of the project is really a version zero state.
So I propose that we change our versioning, existing tags will be shifted to be v0.1.0, v0.1.1, and v0.1.2 respectively. At such a time that the API is stable, we can revisit tagging the project as v1.0.0.
Note: By project, I mean the iOS Proxy, not Core, the Protocol, or any other component of SmartDeviceLink.
Class forwarding (@class
) syntax is used almost nowhere in the project. Many .h import statements can be altered to @class
statements. This should help prevent import loops and reduce compile times.
It can be safely removed
Currently most code is styled incorrectly according to Apple's Coding Guidelines for Cocoa.
I also really like the New York Times Objective-C Style Guile with only a few disagreements.
Prefer properties everywhere.
For instance SDLProxy's private instance variable SDLLockScreenManager should be a private property.
SDLJsonEncoder and Decoder classes add unnecessary complexity and improperly built singletons to call a single line of code
NSData* jsonData = [NSJSONSerialization dataWithJSONObject:dict options:kNilOptions error:&error];
The error is ignored in both cases. It also defines two protocols, that are only used in those classes (Defined in SDLDecoder.h and SDLEncoder.h)
SDLDecoder.h and SDLJsonDecoder can be removed and the line
NSDictionary* rpcMessageAsDictionary = [[SDLJsonDecoder instance] decode:self.payload];
can be replaced in SDLV1ProtocolMessage and SDLV2ProtocolMessage with
NSDictionary *jsonObject = [NSJSONSerialization JSONObjectWithData:msgBytes options:kNilOptions error:nil];
The same can be applied to SDLJsonEncoder, and the one class which calls its encode message
The I syntax is a Java platform syntax. The Cocoa syntax would be SDLProxyDelegate.
I haven't looked deep into this, so I don't know if it's indicative of a larger issue (such as running the socket connection on the main thread). Either way, this should not block the main thread, but should have callbacks of some sort to indicate when a connection or timeout occurs.
Regardless of the type of message, if an SDLRPCMessage
object is initialize using initWithName
, then the messageType
is automatically set to "request".
messageType = NAMES_request;
This should instead set the messageType
to whatever is appropriate based on the message.
A simple way of doing this would to override the initWithName
function in SDLRPCRequest
, SDLRPCNotification
, and SDKRPCResponse
, and set the messageType
in that override instead.
For example, the override in RPCNotification
might look like:
-(id) initWithName:(NSString*) name {
if (self = [super initWithName:name]) {
messageType = NAMES_notification;
[store setObject:function forKey:messageType];
}
return self;
}
The values defined in SDLHMILevel
all use a different name for the function attached to them from the value that they return. They instead use the internal_name
defined for them in the spec, and as a result all of the function names have an "HMI_" prefix attached to them.
+(SDLHMILevel*) HMI_FULL;
+(SDLHMILevel*) HMI_LIMITED;
+(SDLHMILevel*) HMI_BACKGROUND;
+(SDLHMILevel*) HMI_NONE;
This prefix was supposedly necessary at one point in time, but at this point is the only SDLEnum
which follows this naming convention, all others enum values with prefixes in their internal_name
instead use the name
field for their function names.
It's currently sandwiched in the SDLRPCMessage file.
Developer feedback: "The file is actually named FMCHMIZoneCapabilities.h (note the difference in casing), which causes the build to fail on case-sensitive file systems."
There are several enum values which are defined in the spec, but are not currently implemented in the library
Here is a list of all such values:
From what I understand, there are several reasons for this:
I am not entirely certain which values fall under which category, so if someone more familiar with what features have yet to be implemented could have a look at these, then that would be helpful.
Let's Unit Test all of our public API methods. This will help with future changes, and also with open source, since all open source changes can be tested for breakage and add additional tests for new features.
Xcode 6.3 introduces attributes for parameters and properties that indicate nullability, mainly for Swift interop (EDIT: Also for Obj-C interactions!). We'll want to implement that into our APIs to help any Swift developers out there.
#pragma clang assume_nonnull begin
// ...
-(void)registerNib:(UINib *)nib forCellReuseIdentifier:(NSString
*)identifier;
-(nullable UITableViewCell *)cellForRowAtIndexPath:(NSIndexPath)indexPath;
@property (nonatomic, readwrite, retain, nullable) UIView *backgroundView; // ...
#pragma clang assume_nonnull end
Adding nullability annotations to Objective-C APIs does not affect backward compatibility or the way in which the compiler generates code. For example, nonnull pointers can still end up being nil in some cases, such as when messaging a nil receiver. However, nullability annotations—in addition to improving the experience in Swift—provide new warnings in Objective-C if, for example, a nil argument is passed to a nonnull parameter, making Objective-C APIs more expressive and easier to use correctly.
Currently they return mutable arrays, which means that anybody could accidentally modify the array, which could easily result in bugs.
Especially in the case of the putFileRPCRequest, which is already retained as an object association by the inputStream.
The only time this property is retrieved is in the proxy's [stream:handleEvent] method, which has access to the putFileRPCRequest which computes the property.
Is it possible to poll/or get notified when a vehicle is in motion?
The current precompiled header includes UIKit, which is not a framework needed to be imported and accessible in most classes in SDL so it should be removed.
The precompiled header already imports Foundation so Foundation imports can be removed elsewhere.
Anything that is imported in most classes should be moved to the precompiled header instead.
Every setter method in every SDLRPCStruct subclass takes an object and sets it in the store. If the object passed into the method is nil, the object is removed from the store instead (because passing nil to setObject throws an NSInvalidArgumentException). This makes every setter look like
-(void) setNavigationText:(NSString*) navigationText {
if (navigationText != nil) {
[store setObject:navigationText forKey:NAMES_navigationText];
} else {
[store removeObjectForKey:NAMES_navigationText];
}
}
This can be cleaned up by creating a category on NSMutableDictionary and including it in the SDLRPCStruct class. It would look like
- (void)setOrRemoveObject:(id)object forKey:(id <NSCopying>)key {
if (nil != object) {
[self setObject:object forKey:key];
} else {
[self removeObjectForKey:key];
}
}
Then every setter can be reduced to
- (void)setNavigationText:(NSString *)navigationText {
[store setOrRemoveObject:navigationText forKey:NAMES_navigationText];
}
Is there a matrix showing features of SDL and what vehicles/models support which features (and over which transport)?
Everything is written to the file from the thread the method was called from. This could create threading issues with the file, and if written on the main queue, we're now writing to disk from the UI queue, which should be avoided.
I'd propose we create a serial dispatch queue that can write to the file asynchronously, to not block the thread we're logging from, and to prevent threading issues.
The current process is:
Every time the full message is not in the buffer and bytes are received the header is re-parsed. A state machine should be used to parse the incoming bytes.
iOS8 added the ability to create dynamic frameworks - building a Cocoa Touch Framework for developers allows them to use the SDL framework in the exact same way that they use native frameworks.
Foreground app tracking requires the ability to send RPC Notifications to the head unit, we may as well include the ability to send any type of message. This issue will be resolved as part of the PR #33.
The method is currently defined as
- (void)putFileStream:(NSInputStream*)inputStream :(SDLPutFile*)putFileRPCRequest;
and should be something like
- (void)putFileStream:(NSInputStream*)inputStream withRequest:(SDLPutFile*)putFileRPCRequest;
Note: the current method may need to be deprecated in order to avoid compatibility issues and increasing the major version number.
Not having any sort of threading is a separate (larger) issue, but that every property is atomic is going to slow the library down quite a bit. Most (or all, if we do our threading properly) things should be nonatomic.
The parameter key and value constants defined in SDLNames.h should use extern instead of define for safe compile time type checking and to avoid collisions.
Apple's Recommendation suggests
Define constants for strings used for such purposes as notification names and dictionary keys. By using string constants, you are ensuring that the compiler verifies the proper value is specified (that is, it performs spell checking).
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.