PVT solution consistency checks #526
Conversation
A commit to try and test pull request.
This class will hold various spoofing detection functions and other required functions
Necessary modifications to PVT block to instantiate spoofing detector class
Spoofing detector object is created in the rtklib_pvt_gs constructor. Sub-config parameter's reference is passed to the object.
PVT consistency checks PVT consistency checks PVT consistency checks Added conf file
There was a problem hiding this comment.
I need to start with my standard disclaimer that I have no authority on this project, so feel free to disregard my comments if you must.
Since this is a GSoC project, I'll add to that that you may not be used to writing production-quality code, and you've probably never had as detail-oriented a reviewer as me look at your code before. Please be aware that comments on the code are not comments on you. Finally, your new/modified code is evaluated on its own merits. You may completely follow an existing pattern; since I'm not looking at the pattern and only your mods, poor C++ practices will be criticized. Those may be the kinds of comments you choose to ignore, or at least file away for future reference.
This is an ambitious and far-reaching PR, touching a lot of files. I would have liked to seen smaller chunks; because there is so much, and it all builds on some basic decisions, it is hard to address the issues. While there are numerous code quality issues, focusing on them is the wrong priority when there are design concerns. What I find lacking is an overarching vision of how your tests detect spoofing. I see various counters, but it's not obvious what phenomena you are detecting.
Spoofing detection should be rooted in physics that the spoofer can't override. Comments are your friend. It would be extremely useful for each of your tests to describe what they are testing, why it's an indication of spoofing, and how the test is designed. As noted in the position-related tests, it appears that there is an assumption that the receiver isn't moving. That's not likely to be a useful check, since a stationary receiver should be considered an edge case.
I would encourage you to read anything from Scott Meyers that you can. Another priceless resource is Herb Sutter. Have friends read your code and tell you what they think it does. The ability of code to be read is a key factor in maintainability. Especially in an open-source project, where the next person to touch your code may be continents away from you, it is important that your code clearly communicates your intent, both to the reader and to the computer. Towards that end, meaningful variable names, and functions that are of the form verbNoun are important, with returns that match the intuition.
For example, void position_jump() implies that it will "jump" the position (whatever that means). Even if it were check_position_jump(), one would expect a "check" function to return the results of the check. Perhaps a more natural construct would be bool didPositionJump(), which would return true if the position jump is detected.
This doesn't feel like "ready to ship" code; it reads more like a homework assignment. You can improve the quality by refining the design, further isolating your changes from the existing PVT and by more clearly defining precisely what it is you are testing for, and why that's a meaningful test. See abnormal_position_checks(); it computes a score that appears largely random.
Don't forget to use the tools available to check your code, like clang-format and clang-tidy. They will get run before this can be merged anyway, so you might as well run them now and fix what they find.
|
|
||
| ;######### SIGNAL_SOURCE CONFIG ############ | ||
| SignalSource.implementation=File_Signal_Source | ||
| SignalSource.filename=./data/overpowered_pos_jump_2M.dat |
There was a problem hiding this comment.
unless this file is going to be provided, is this config file really something we need to control?
|
|
||
|
|
||
| ;######### SIGNAL_CONDITIONER CONFIG ############ | ||
| SignalConditioner.implementation=Signal_Conditioner |
There was a problem hiding this comment.
if the sub-components are Pass-Through, why not just make the whole conditioner Pass-Through and elide the sub-sections?
| Acquisition_1C.item_type=gr_complex | ||
| Acquisition_1C.coherent_integration_time_ms=1 | ||
| Acquisition_1C.pfa=0.01 | ||
| ;Acquisition_1C.pfa=0.01 |
There was a problem hiding this comment.
delete? This commented line appears to duplicate the previous line
|
|
||
| ;######### TELEMETRY DECODER GPS CONFIG ############ | ||
| TelemetryDecoder_1C.implementation=GPS_L1_CA_Telemetry_Decoder | ||
| TelemetryDecoder_1C.dump=true |
There was a problem hiding this comment.
I wouldn't expect any of the blocks to have "dump=true" for production
| SecurePVT.max_ground_speed=200 | ||
|
|
||
| ; Velocity | ||
| SecurePVT.pos_error_threshold=5 ; Receiver will decrease spoofer score if the new location is within 5m of the lkgl, used for velocity consistency check |
There was a problem hiding this comment.
newline terminate the final line
| // Calculate the result | ||
| distance = distance * R; | ||
| return distance; | ||
| } |
| int d_spoofer_score; | ||
| double d_assurance_score; | ||
|
|
||
| const Gnss_Synchro** d_gnss_synchro; |
There was a problem hiding this comment.
don't use naked pointers; use shared_ptr instead
|
|
||
|
|
||
| // Collection of PVT consistency checks | ||
| class PVTConsistencyChecks |
There was a problem hiding this comment.
these classes/structures should be in files named more consistently with their contents. It's true that this project doesn't directly have names that match the classes, but for example, this class should be in header named "pvt_consistency_checks.h" by the project convention
| Position Jump - 1 | ||
| Compare Velocity - 2 | ||
| Static Position - 3 | ||
| */ |
| @@ -1429,6 +1438,49 @@ void dll_pll_veml_tracking::log_data() | |||
| // PRN | |||
| uint32_t prn_ = d_acquisition_gnss_synchro->PRN; | |||
| d_dump_file.write(reinterpret_cast<char *>(&prn_), sizeof(uint32_t)); | |||
|
|
|||
| // Spoofing detection part - need to move this to a suitable location later. | |||
There was a problem hiding this comment.
"later" should be before this is merged?
A collection of functions to ensure consistency of obtained PVT solution to detect GNSS spoofing attacks.
These include:
Consistency checks 1 through 4 are implemented in the PVT block and check 5 is implemented in the tracking block
A score-based system is implemented to indicate the reliability of the calculated PVT solution.