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

Rewrite #23

Merged
merged 21 commits into from Aug 19, 2015
Merged

Rewrite #23

merged 21 commits into from Aug 19, 2015

Conversation

beltex
Copy link
Owner

@beltex beltex commented Aug 3, 2015

Sorry about the monolithic first commit of the PR. Started just tinkering with things on the side, so commit history would have been quite sporadic. This was supposed to be a refactoring, but turned into a rewrite. It was a good time to do this with Swift 2.0. Tagged version 0.0.15 to be a marker release before these breaking changes are merged in.

Key Changes

  • Extensions for types to do encoding and decoding which should be much cleaner
  • Using the new error handling in Swift
  • All static methods. Shouldn't be a need to have more than one connection to the SMC in a process
  • All public now. Easily extendable and flexible, so the client can use it how they like. Have access to lower level functions to make custom calls (callDriver), or just stick to higher level functions (temperature(), fanSpeed(), etc.)
  • Easy to add new temperature sensor with the way things are structured now (not an enum any more)

Error Handling

Wanted to highlight error handling because a lot of time was spent thinking about it. Keep in mind that since we are dealing directly with hardware here, any call to the SMC could fail for any number of reasons.

Certainly, the existing approach of a tuple return with two return codes (I/O Kit and SMC), was subpar for a number of reasons. First off, should have abstracted the two error codes into one at the very least. Second, was not using optionals for the main value, which mean't having to document return values on error, which leads to all sorts of issues. Opted to go with the new error handling in Swift 2.0. We'll see how it goes. I know throwing and catching exceptions all over the place can be a pain, but I think ultimately it led to the most clarity and simplicity. The key question was, do we only care about just success/failure, and thus could simply use an optional return type, or does the client indeed care about the error in question? Thats what #8 tried to bring up (that was before error handling was added to Swift). Ultimately, I thought the answer is yes, because in many cases, the client can act on the error. There is a range of possible errors that could occur, from key not found (which would be common), to things like lack of privilege (needing root to write) for example. Nonetheless, the possibility of a try? in the future (see here) adds extra flexibility when the client wants to just check for success/failure (folks have come up with their own implementations of this in the meantime). Finally, I know Result has been popular, and thats certainly something that could be looked at in the future.

@beltex beltex changed the title [WIP] Rewrite Rewrite Aug 15, 2015
Think this addresses #15. So it's now -1 degree Celsius. My theory on this is
that certain sensors at lower temperatures have inaccurate readings. Maybe there
located close to an internal fan, and the airflow is causing it to be skewed? In
any case, this at least lets us feel good that the machine is not in danger of
over heating.

#15
This makes the cross check lookup easy for allUnknownTemperatureSensors()
No longer pre-sorted due to use of dictionary
Seems to run a bit slow though. This is likely due to the manner in which
we have to get the keys (via SMC index). Would be nice to cache them some how,
but that might be overkill.

http://github.com/beltex/SMCKit/issues/17
@beltex
Copy link
Owner Author

beltex commented Aug 17, 2015

PR description updated.

@perfaram
Copy link
Contributor

Nice !

@beltex
Copy link
Owner Author

beltex commented Aug 18, 2015

Thanks! :)

beltex added a commit that referenced this pull request Aug 19, 2015
@beltex beltex merged commit 9a41124 into master Aug 19, 2015
@beltex beltex deleted the rewrite branch August 19, 2015 13:48
@beltex
Copy link
Owner Author

beltex commented Aug 25, 2015

Follow up.

The try? functionality that was mentioned earlier has been added to the just released Xcode 7 Beta 6. From the release notes:

A new keyword 'try?' has been added to Swift. 'try?' attempts to perform an operation that may throw. If the operation succeeds, the result is wrapped in an optional; if it fails (I.e. if an error is thrown), the result is 'nil' and the error is discarded. ‘try?’ is particularly useful when used in conjunction with “if let” and “guard”.

Should be helpful, will explore.

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

Successfully merging this pull request may close these issues.

None yet

2 participants