It seems necessary to review the logic used in the configuration of the u-blox driver in the ubx.cpp file for X20 #192
Description
I am testing the ublox X20P module and, while reviewing the driver, I have detected some inconsistencies in the code.
For example, in
GPSDriverUBX::configureDevice
in the activation of the Galileo system we have:
`
if (config.gnss_systems & GNSSSystemsMask::ENABLE_GALILEO) {
UBX_DEBUG("GNSS Systems: Use Galileo");
cfgValset<uint8_t>(UBX_CFG_KEY_SIGNAL_GAL_ENA, 1, cfg_valset_msg_size);
if (_board == Board::u_blox9_F9P_L1L2 || _board == Board::u_blox_X20) {
UBX_DEBUG("GNSS Systems: Use Galileo E5B");
cfgValset<uint8_t>(UBX_CFG_KEY_SIGNAL_GAL_E5B_ENA, 1, cfg_valset_msg_size);
} else if (_board == Board::u_blox9_F9P_L1L5 || _board == Board::u_blox10_L1L5 || _board == Board::u_blox_X20) {
UBX_DEBUG("GNSS Systems: Use Galileo E5A");
cfgValset<uint8_t>(UBX_CFG_KEY_SIGNAL_GAL_E5A_ENA, 1, cfg_valset_msg_size);
}
if (_board == Board::u_blox_X20) {
UBX_DEBUG("GNSS Systems: Use Galileo E5B");
cfgValset<uint8_t>(UBX_CFG_KEY_SIGNAL_GAL_E5B_ENA, 1, cfg_valset_msg_size);
UBX_DEBUG("GNSS Systems: Use Galileo E6");
cfgValset<uint8_t>(UBX_CFG_KEY_SIGNAL_GAL_E6_ENA, 1, cfg_valset_msg_size);
}`
It seems that the idea was, but:
F9P L1L2 → Galileo + E5B
F9P L1L5 / u-blox10 L1L5 → Galileo + E5A
X20 → Galileo + E5A + E5B(not supported) + E6
From the docs of X20P we can see:
From the documentation, we can see the systems and bands compatible with the X20P module, and we can conclude that in the code, apart from not having a clear structure, the E2B band is activated twice when this module does not have it.
As a proposal, a switch-case structure in the code could be recommended, applying an independent configuration to each module.
Furthermore, I don't know if E1 C/B is activated. I don't know if it is activated by default when Galileo is activated or if it has to be done explicitly.
Something similar happens with BeiDou: B2I is activated when it is not actually supported, and B1 I/C is not explicitly activated.
Although Glonass is not supported by X20P, it is included in part of the internal logic of the enablement.
While reviewing the code, I had the feeling that it wasn't robust, hence my motivation for raising this issue.