{"action":"created","issue":{"url":"https://api.github.com/repos/diydrones/ardupilot/issues/2599","labels_url":"https://api.github.com/repos/diydrones/ardupilot/issues/2599/labels{/name}","comments_url":"https://api.github.com/repos/diydrones/ardupilot/issues/2599/comments","events_url":"https://api.github.com/repos/diydrones/ardupilot/issues/2599/events","html_url":"https://github.com/diydrones/ardupilot/pull/2599","id":96926824,"number":2599,"title":"Force compasses to use SI units","user":{"login":"staroselskii","id":4541230,"avatar_url":"https://avatars.githubusercontent.com/u/4541230?v=3","gravatar_id":"","url":"https://api.github.com/users/staroselskii","html_url":"https://github.com/staroselskii","followers_url":"https://api.github.com/users/staroselskii/followers","following_url":"https://api.github.com/users/staroselskii/following{/other_user}","gists_url":"https://api.github.com/users/staroselskii/gists{/gist_id}","starred_url":"https://api.github.com/users/staroselskii/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/staroselskii/subscriptions","organizations_url":"https://api.github.com/users/staroselskii/orgs","repos_url":"https://api.github.com/users/staroselskii/repos","events_url":"https://api.github.com/users/staroselskii/events{/privacy}","received_events_url":"https://api.github.com/users/staroselskii/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/diydrones/ardupilot/labels/AllVehicles","name":"AllVehicles","color":"bfd4f2"},{"url":"https://api.github.com/repos/diydrones/ardupilot/labels/BUG","name":"BUG","color":"fc2929"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2015-07-23T23:35:19Z","updated_at":"2015-08-20T22:28:19Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/diydrones/ardupilot/pulls/2599","html_url":"https://github.com/diydrones/ardupilot/pull/2599","diff_url":"https://github.com/diydrones/ardupilot/pull/2599.diff","patch_url":"https://github.com/diydrones/ardupilot/pull/2599.patch"},"body":"A replacement for #2397. This one needs extensive testing. \r\n\r\nThe code for HMC was slightly refactored. Nothing fancy, just extracted a couple of methods. If that's not needed, let me know!) . There's a couple of places in the HMC's code that I don't entirely understand. Especially this:\r\n```c++\r\n    _mag_x = -rx;\r\n    _mag_y =  ry;\r\n    _mag_z = -rz;\r\n```\r\nWhy do we do such a thing? \r\n\r\nAnd also this one:\r\n```c++\r\nif (_product_id == AP_COMPASS_TYPE_HMC5883L) {\r\n        field.rotate(ROTATION_YAW_90);\r\n}\r\n```\r\n\r\nIs it absolutely necessary to rotate on the low level? Cannot it be done on the higher one? \r\n\r\nMagnetometer offsets also need to be adjusted (I suggest using something like 50 uT. That's the magnetic field vector around the Earth's surface).\r\nI also need some insight on PX4 compass driver. \r\n\r\nUpdated (9d9068d is the previous HEAD):\r\n- Improved constness in HMC code\r\n\r\nRebased on master (939126671 is the previous HEAD)"},"comment":{"url":"https://api.github.com/repos/diydrones/ardupilot/issues/comments/133200340","html_url":"https://github.com/diydrones/ardupilot/pull/2599#issuecomment-133200340","issue_url":"https://api.github.com/repos/diydrones/ardupilot/issues/2599","id":133200340,"user":{"login":"lucasdemarchi","id":31864,"avatar_url":"https://avatars.githubusercontent.com/u/31864?v=3","gravatar_id":"","url":"https://api.github.com/users/lucasdemarchi","html_url":"https://github.com/lucasdemarchi","followers_url":"https://api.github.com/users/lucasdemarchi/followers","following_url":"https://api.github.com/users/lucasdemarchi/following{/other_user}","gists_url":"https://api.github.com/users/lucasdemarchi/gists{/gist_id}","starred_url":"https://api.github.com/users/lucasdemarchi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lucasdemarchi/subscriptions","organizations_url":"https://api.github.com/users/lucasdemarchi/orgs","repos_url":"https://api.github.com/users/lucasdemarchi/repos","events_url":"https://api.github.com/users/lucasdemarchi/events{/privacy}","received_events_url":"https://api.github.com/users/lucasdemarchi/received_events","type":"User","site_admin":false},"created_at":"2015-08-20T22:28:19Z","updated_at":"2015-08-20T22:28:19Z","body":"I think it would be easier to add `get_field_milligauss()` and `get_offsets_milligauss`, leaving `get_field()` and `get_offsets`as is. So while the first returns the value converted to milligauss() the second is the raw value, in sensor units. Then you update the callers in the places that a) are not saved to storage and are not sent via mavlink.\r\n\r\nThe constants in motors.cpp can be updated, as long as you update compass.get_offsets() to be compas.get_offsets_milligauss().\r\n\r\n@tridge @rmackay9. Does it seem reasonable to you?"}}