-
Notifications
You must be signed in to change notification settings - Fork 131
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
Update to allow mbed and the mbed online compiler to build. #449
base: master
Are you sure you want to change the base?
Changes from all commits
7bf0c72
f2baf1e
5b015a3
4f1a31f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -226,8 +226,8 @@ CompassCalibration MicroBitCompassCalibrator::spherify(Sample3D centre, Sample3D | |||
Sample3D MicroBitCompassCalibrator::approximateCentre(Sample3D *data, int samples) | ||||
{ | ||||
Sample3D c,t; | ||||
Sample3D centre = { 0,0,0 }; | ||||
Sample3D best = { 0,0,0 }; | ||||
Sample3D centre; | ||||
Sample3D best; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default constructor initialises the values to zero, but is there any specific reason to remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the other initializer removals. I will remove the whitespace. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the constructor code has been reverted (was there constructor code to initialise these to 0?), shouldn't these 3 lines be reverted as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There wasn't constructor code to initialise these as they are initialised to zero by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @geowor01 I don't think there are any guarantees that struct fields are zero initialised unless they are placed in bss (globals). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jamesadevine The default constructor sets the value to zero. microbit-dal/inc/types/CoordinateSystem.h Line 81 in 8c2403e
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @geowor01 Ah cool 😄, my bad! |
||||
|
||||
float score; | ||||
|
||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no experience with armcc, but this is a really weird-looking and cryptic symbol, is there perhaps a better one for this value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know, I extracted this symbol from https://github.com/lancaster-university/mbed-classic/blob/master/targets/cmsis/TARGET_Freescale/TARGET_K20XX/TARGET_K20D50M/TOOLCHAIN_ARM_STD/sys.cpp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see this is also defined here: https://github.com/lancaster-university/mbed-classic/blob/master/targets/cmsis/TARGET_NORDIC/TARGET_MCU_NRF51822/TOOLCHAIN_ARM_STD/sys.cpp#L15
As weird looking at it seems, I guess it is what it is ¯\_(ツ)_/¯