-
Notifications
You must be signed in to change notification settings - Fork 50
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
pre arm checks: SN number add region parameter #101
base: master
Are you sure you want to change the base?
pre arm checks: SN number add region parameter #101
Conversation
@friissoren @tridge @Davidsastresas Do you have feedback regarding this improved pre-arm check? In general there are three kind of checks:
There are other PRs that relate a bit to this: #102 and #103 |
On QGC front I can accommodate it to whatever you guys do here and in Ardupilot. Just let me know what is necessary and I will include. Thanks! |
@@ -37,6 +37,7 @@ const Parameters::Param Parameters::params[] = { | |||
{ "PUBLIC_KEY4", Parameters::ParamType::CHAR64, (const void*)&g.public_keys[3], }, | |||
{ "PUBLIC_KEY5", Parameters::ParamType::CHAR64, (const void*)&g.public_keys[4], }, | |||
{ "MAVLINK_SYSID", Parameters::ParamType::UINT8, (const void*)&g.mavlink_sysid, 0, 0, 254 }, | |||
{ "REGION", Parameters::ParamType::UINT8, (const void*)&g.region, 0, 0, 3 }, // 0 = world, 1 = USA, 2 = Japan, 3 = EU |
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.
should use an enum for this
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.
implemented an enum, but it does not compile, need help from @tridge. The enum is not known in the RemoteIDModule.ino. The code also needs a rebase.
09dc07e
to
93f93f3
Compare
93f93f3
to
4d71ebf
Compare
@BluemarkInnovations I've restructured this to be cleaner and easier to follow. Please check! |
@BluemarkInnovations any progress on this? |
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 like the restructuring of the code! Much better now.
I tried to compile this updated code and test it. It compiles, but somehow the parameter list was not shown anymore in DroneCAN GUI tool or MissionPlanner. This limited my tests, but looking to the code, everything seems okay to me.
switch (region) { | ||
case Region::EU: | ||
return REG_REQUIRE_LOC | REG_REQUIRE_BASIC_ID | REG_REQUIRE_OPERATOR_ID | REG_REQUIRE_OPERATOR_LOC | REG_REQUIRE_OPERATOR_ID | REG_REQUIRE_SERIAL_NUM; | ||
case Region::JAPAN: |
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.
For now I would set Japan to WORLD. According to this page, two basic ID messages are needed, but also authentication messages. The latter is not implemented in ArduRemoteID. Therefore better to set it to WORLD, otherwise users may assume the firmware is compatible with Japan
WORLD=0, | ||
USA=1, | ||
JAPAN=2, | ||
EU=3, |
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.
Based in on my experience, in some countries of the EU, also a CAA ID is mandatory for companies that fly drones. In those countries: there are 3 mandatory fields for companies: a) Serial Number b) CAA ID (company ID) c) pilot (operator ID). For now I would suggest to leave it as it, as I don't have a good overview or source that lists the requirements per EU country.
This PR adds: