-
Notifications
You must be signed in to change notification settings - Fork 85
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
Add new SignalK sensor: magnetometer heading #145
Conversation
edits to platformio.ini to add libraries new file: src/sensors/orientation_9DOF_input.cpp new file: src/sensors/orientation_9DOF_input.h new file: src/sensors/sensor_NXP_FXOS8700_FXAS21002.cpp new file: src/sensors/sensor_NXP_FXOS8700_FXAS21002.h new file: examples/FX8700_heading_example.cpp (for use as main.cpp)
Hi! Now that's an interesting sensor you've got there! Sorry for the delay in my response; I initially glossed over the PR and thought it was still work in progress, but after a closer look it seems ready for review. :-) |
@@ -8,37 +8,30 @@ | |||
; Please visit documentation for the other options and examples | |||
; https://docs.platformio.org/page/projectconf.html | |||
|
|||
[env:d1_mini] |
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.
Could you drop out the platformio.ini changes? Or, rather, ensure that the default platformio.ini is able to compile simple projects.
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.
Sounded simple...but I ended up pulling hair for a week trying to get the libraries sorted out. Seems that PlatformIO (at least on my installation) doesn't always respect the entries in library.json when fetching the needed dependencies. I'm still unsure why that's happening, however my present versions of library.json
and platformio.ini
result in the project compiling fine for the existing three boards (d1_mini, esp32dev, and esp-wrover-kit).
I took the opportunity to edit platformio.ini to put the common configuration elements together (under the [env]
tag), while the board-specific elements are under appropriate [env:board_name]
tags. Now switching between boards is as simple as uncommenting the desired board at the top of the file, in default_envs =
The specific items in platformio.ini added to support the orientation sensor are:
lib_deps =
now includes Adafruit AHRS and library 7212 (Software Serial)build_flag =
for each of the three boards includes aI./.pio/libdeps/board_name/SoftwareSerial/
entry, to fix a compiler error in which an Adafruit library can't find SoftwareSerial.h.
The library.json
file has two new entries: Adafruit AHRS, and SoftwareSerial. I realize this duplicates what is in the platformio.ini file, but it doesn't do any harm and it's probably a good way of documenting the library dependencies. Also, if anyone elses IDE properly parses library.json but doesn't need/use platformio.ini then the project should still compile OK.
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 had similar problems with libraries, but I eventually got them all to work by being ONLY in library.json. I may have done a "Clean" in PlatformIO, or I may have deleted the entire project directory and started over again - I don't remember. But I'm pretty sure you shouldn't need Adafruit AHRS in the platformio.ini. I don't know about the 7212 library, but I'm guessing it's the same situation - that is, it shouldn't need to be in platformio.ini, only in library.json. I wish I could give you more guidance.
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 agree that is shouldn't be necessary to list the libraries in platformio.ini, but I haven't been able to get it to work on my system without doing so. I've tried many Cleans, deleting the .pio folder, even reinstalling PlatformIO - all without success. I'll continue sporadically to try things as I or others think of them, but in the interest of making progress I need to keep the libraries in platformio.ini for now. It's frustrating... :-)
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.
As noted here #145 (comment), I've managed to get everything happily compiling without duplicating the library dependencies in platformio.ini. Thanks for your patience :-)
I have retained the changes to platformio.ini
making it easier to switch between cofigurations for the three boards that seem to be tested to date.
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.
(Sorry about the longish delay - I was quite preoccupied with other things.) Could you please revert platformio.ini default settings back to as they were. I don't want anyone's builds to suddenly break once they update their repo. EDIT: I originally suggested to comment out the additions because I only noticed the first ones. The espressif32_base and related sections are very good additions and should definitely stay! :-)
FWIW, our current approach for managing platformio.ini is a bit unoptimal. Many (most?) people will need to do local modifications to work on SensESP and then those changes will need to be separately maintained and tend to find their ways to PRs. Like in this case. I'll open an issue about that.
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.
OK - how about this version: BjarneBitscrambler@149aeb9#diff-237285c047dad808f73698c139144e1a I've changed the default board to d1_mini, and re-commented the three settings for the esp32dev board:
So, now when folks pull down the changed platformio.ini I believe one of these situations will apply:
- they are using a d1_mini, with no changes from the current SensESP platformio.ini. No action needed, as the new .ini file defaults to d1_mini with unchanged settings.
- they are using a d1_mini, with some locally changed settings. These will have to be manually added to the d1_mini block.
- they are using an esp32dev board, or esp-wrover-kit. The proper
default_envs =
board will need to be selected, and any other local setting changes made. - they are using a new type of board. Add the board to the
default_envs =
list, and append a new block of settings.
Except for option (1.) some manual intervention will be needed. But I think that will be the case regardless of how trivial a change is made to platformio.ini
, since pulling in an updated version requires the user to at least select the correct default board and/or apply local changes. However, with settings grouped with their associated board and an easy way to select which board is being used, it seems clearer than the existing platformio.ini where one has to search out and copy sections.
An alternative, is to make no edits to platformio.ini
with this PR, but to change the name as you suggested in #153 and to introduce the changes that way. Would you prefer that approach - is that what you meant by "revert platformio.ini default settings back to as they were"?
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.
Your first proposed approach looks good to me! It would keep the d1_mini as the default which would probably still serve the majority of users/developers best. But additionally, your improved organization of the esp32 specific parts looks handy. As for the esp32 specific options, I readily admit I don't have experience on them. If you think some of them should be enabled by default, that's fine by me! As long as it's a conscious decision. :-)
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 personally found the three esp32-specific options:
build_unflags = -Werror=reorder
board_build.partitions = no_ota.csv
monitor_filters = esp32_exception_decoder
to be useful (in fact the no_ota.csv option was essential in order to fit the firmware image in memory). So, let's enable those for the esp32 boards. I'll turn those on again and re-commit...
How much processing power does the sensor fusion algorithm require? Would the library work with ESP8266 as well or is ESP32 required? If it's ESP32-only, it'd be good to make a note of that somewhere. Other than the |
per review by mairas in SensESP PR#145, where possible the Serial.print() calls were replaced by debugI(),debugE(),etc calls for more flexibility.
using radians (instead of degrees) to agree with SignalK conventions. The conversion constant SENSORS_RADS_TO_DPS is defined in the Adafruit AHRS library.
Not as much as I thought... I benchmarked by timestamping the entry and exit of the
Therefore the I2C read of the two ICs takes about (51-17)/20 = 1.7 ms, and each call to the As a final note, the compiled program size without the orientation sensor listener is 1340341 bytes, whereas with the orientation sensor is 1374621 bytes, so the orientation sensor increases the executable size by about 34300 bytes on my setup. |
Compiles for espressif8266 d1_mini; espressif32 esp32dev; espressif32 esp-wrover-kit. Some duplicated ilibraries in library.json and platform.ini to overcome IDE shortcomings
Compiles for espressif8266 d1_mini; espressif32 esp32dev; espressif32 esp-wrover-kit. Some duplicated ilibraries in library.json and platform.ini to overcome IDE shortcomings
I made three minor comments. Since @mairas did the real review, I will let him merge it when he feels all his concerns have been addressed. |
Excellent - thanks Brian. I definitely will look at the examples to take
maximum advantage of all the data the sensor provides.
…On Sun, Aug 30, 2020 at 9:06 PM Brian Smith ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/sensors/orientation_9DOF_input.cpp
<#145 (comment)>:
> + // TODO - process full return vals. Right now does nothing useful.
+ output = -1.0;
+ break;
+ case (compass_hdg):
+ output = pSensorFXOSFXAS->getHeadingRadians();
+ break;
+ case (gyro):
+ // TODO - process full return vals. Right now does nothing useful.
+ output = -2.0;
+ break;
+ default:
+ output = -3.0;
+ }
+ notify();
+}
+
When you're ready to do something with these two other values
(accelerometer and gyro), look at how the BME280 sensor works: a single
physical sensor, with three possible outputs, each connected to its own SK
Path.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#145 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQJATFPGTIJGYVXJUGUYUODSDMOUVANCNFSM4QBZJXKA>
.
|
There are two libraries with the name SoftwareSerial (#7212, #2728) neither of which is the proper choice for ESP-based boards. The proper one is EspSoftwareSerial, SignalK#168
- Removed Adafruit AHRS and SoftwareSerial libraries - Removed build_flag that included SoftwareSerial.h search path, since proper installation of EspSoftwareSerial has the correct compiler search path. - Collected configuration settings common to Espressif32 platform in single section, which is then incorporated into multiple boards using extends = syntax
Happiness - The orientation sensor code is compiling without needing to duplicate the Adafruit AHRS and SoftwareSerial libraries in both Remaining IssueArduinoJson library version 6 gets pulled in by one of the Adafruit libraries, which causes both version 5 (called for in SensESP Longer storyNote that I changed the SoftwareSerial library to EspSoftwareSerial. It turns out that neither of the two libraries going by the name SoftwareSerial are the proper ones to use. Library ID #7212 tags itself as being compatible with Espressif8266 and Espressif32 platforms, but its SoftwareSerial.h has two #includes for avr/* files, which aren't present in the espressif environment. ID #2728's description says that it is "ALPHA WORK IN PROGRESS. DO NOT USE IN PRODUCTION CODE" It is the latter library that gets installed by default if one doesn't call explicitly for 7212. When 7212 is installed (as I had earlier tried), the compiler complains when it hits Adafruit BluefruitLE nRF51: can't find SoftwareSerial.h The 'fix' for that was to add a build_flag adding the folder location of the 7212 SoftwareSerial.h - I speculate that because this library wasn't compatible with the espressif platform then it wasn't automatically added to the compiler search path. Once the EspSoftwareSerial library was substituted, the compiler error went away and the extra build_flags = -I./.pio/libdeps/[...]/SoftwareSerial/ wasn't needed. |
Nice work on the SoftwareSerial issue! That must have taken some digging. @mairas - this one is now all yours to merge, after reviewing the responses to your review comments. |
-Added ability to pass any of 9-DOF parameters (magnetometer x,y,z; accelerometer x,y,z; gyroscope x,y,z) into SignalK stream.
I had a look at both the GPS and the BME280 examples to see how multiple SignalK streams can be generated from a single sensor - thanks for the pointers, they made it clear. I've now updated the sensor files and the example to demonstrate passing heading, roll, and pitch data from the one sensor. I've also added commented-out sections showing the passing of acceleration in 3-axes (useful for detecting if your boat has tipped over while on the hard), and rotation rate in 3-axes. All 9 parameters have been tested and seem to work. Differences from GPS and BME280The approaches of these 2 sensors are subtly different from what was needed for the orientation sensor, and I thought it useful to explain how/why for others who are implementing new sensors.
By contrast, the orientation sensor works best when all 9 orientation parameters are read simultaneously (or at least as simultaneous as practical) and at a constant rate. The reason is that a software filter processes the data to synthesize values for parameters like compass heading that are more accurate than if just the magnetometer reading alone was used. I haven't dived into the depths of the filter, but I believe it accomplishes this by combining the fast response rate of the gyro data, with the slower but absolute positioning of the magnetometer, with the precision of the accelerometer. Anyway, to ensure that the filter runs at predictable rate, I tied the sensor I2C transaction to the compass heading reading, so whatever rate the user specifies for that parameter, that is also the rate that the filter runs at. All the other parameters are pulled from variables that are updated each time the filter runs. So, for example, if heading updates are set up for every 100ms, then the fastest that any of the other parameters is updated at is 100ms. You can request more frequent SignalK updates, but the data itself will only be refreshed every 100ms. I hope that's clear... Further WorkI haven't done any performance/accuracy testing. Once I put together a more robust prototype I'll experiment with checking things like heading accuracy, response rate, etc. The orientation sensor should be usable as-is however for anyone who wants to start experimenting. I feel this sensor implementation is ready for merging, pending any final/further comments that you may have. Thanks for all your help so far! |
Thanks for the extremely well documented example! I'm sure others will appreciate it as much as I do. One question: why set the sampling rate to 100ms for heading, but all the others to 5 times that? Why not just set them all to the same: 500ms? |
Hi Brian - thanks, I learned a lot pulling this code together. Kudos to you and the other contributors for creating a good framework to hang it all on. My choice of 100ms for the compass heading was based solely on a practical need: I want to provide our radar with heading data so it can overlay the radar returns on top of charts, track targets, etc. The specs on our particular radar (and most of the other models I looked at) say that it needs a 10 Hz update rate for best results. The other parameters are ones that human crew might be interested in, as we don't have any electronics that needs the data, so a slower update rate seemed appropriate. It's all pretty arbitrary, and I've tested update rates as fast as 100 Hz / 10ms and that seemed to work (#145 (comment) documents some of the benchmarking I tried). Alas, I have no better way of visualizing the data than the bundled SignalK Instrument Panel. Your concept of a visual representation of the vessel in 3D space sounds like just the ticket, so if you make any progress in that vein I'd be excited to hear more! I know from looking at Adafruit's product tutorials that they show a 3D-rendered rabbit (or hare, maybe :-) spinning in response to a handheld orientation sensor. That might provide a starting point... When the rabbit starts vomiting, you know the comfort-factor threshold has been exceeded (sorry!) |
Great work! AFAIK, the SoftwareSerial library is only used with the gps senso. The GPS module I was using, and I believe that applies in general, was transmitting its data over a serial interface. I have a dim recollection it took me some time to find a software serial library that was working at all. Apparently didn't find the right one or things have improved since. Glad you found that one, anyway! I only have the final request that the default settings in platformio.ini should be kept as is. |
FYI, here's the |
Retained ability to specify a default board, and automatically select from settings particular to each board. Concern was to not break existing compiles when using new platform.ini, so default board is set to d1_mini. Settings for esp32dev is unchanged. Additions for esp_wrover_kit are retained.
- the esp32 monitor helps provide a call traceback when device resets - the no_ota.csv memory mapping allows the firmware footprint to fit into available flash. - some compiler warnings are suppressed with build_unflags, but this option isn't mandatory.
Thanks a lot for your patience! Merging! |
Sweet! Thanks, Matti for all your help!
…On Tue, Sep 8, 2020 at 12:53 AM Matti Airas ***@***.***> wrote:
Thanks a lot for your patience! Merging!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#145 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQJATFP44UDGQ67LTB65D5DSEXPGTANCNFSM4QBZJXKA>
.
|
Hi folks, I've drafted files to support a new sensor to supply compass heading data. The code works with a combination NXP FXOS8700 and FXAS21002 magnetometer/accelerometer/gyroscope, providing 9-Degrees-of-Freedom orientation information. These ICs are found on products such as the Adafruit Precision NXP 9-DOF Breakout Board, product ID 3463.
New & Changed Code
There are 4 new files to support the sensor, an example main.cpp, and a few changes to platformio.ini for additional external libraries:
src/sensors/orientation_9DOF_input.cpp
to derive classes from the SensESP Sensor and NumericSensor classessrc/sensors/orientation_9DOF_input.h
src/sensors/sensor_NXP_FXOS8700_FXAS21002.cpp
for interface details particular to the FXOS8700 + FXAS21002src/sensors/sensor_NXP_FXOS8700_FXAS21002.h
platformio.ini
to add libraries and to configure for the ESP32examples/FX8700_heading_example.cpp
Why
My motivation for obtaining compass heading data is that many radar features (e.g. target tracking, chart overlay) require accurate vessel heading readings at a fairly quick update rate (e.g. 10 Hz). Flux-gate, Hall-effect and other magnetometer types when used alone are susceptible to errors from pitching and rolling. A sensor combination that includes an accelerometer and gyroscope can, with proper processing, extract more accurate heading data.
Sensor Hardware and Software
I chose the FXOS8700 + FXAS21002 combination because these two high-performance NXP ICs are readily available on an easy-to-use breakout board - Adafruit product #3463. NXP has written a SensorFusion filter algorithm that uses the 9-DOF data to generate orientation readings, and Adafruit has adapted and integrated that algorithm into their AHRS (Attitude and Heading Reference System) library. It was fairly straightforward to employ the Adafruit library in making this SensESP orientation sensor.
The 9-DOF orientation sensor requires calibration before one can obtain reliable readings. The immediate environment (enclosure, nearby ferrous objects, etc) of a magnetometer influences its readings, as does individual IC variations and physical stresses from soldering, and these need to be corrected for. Fortunately, PJRC (linked to in Adafruit's motion sensor tutorial) has written a Motion Sensor Calibration Tool that accepts a serial data stream and generates the calibration values. Adafruit's AHRS
library is compatible with this external tool. More details are online, and in the comments of the relevant source files.
Testing and TODOs
I've tested this code using the Adafruit breakout board connected to an ESP-WROVER-KIT (ESP32) development board. Preliminary impression indicates it should be suitable as a high-speed vessel heading sensor. Further work I foresee includes:
To amplify on this last item: I can imagine it would be interesting to get readings on pitch and roll and rate-of-turn, in addition to just compass heading. These parameters are all available from the same sensor combination. My code provides only heading. Hopefully someone has insights into how we can pull multiple orientation parameters from a single sensor.
I look forward to feedback & improvements on this new sensor. Thanks!