Skip to content
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

Set and Get WiFi configuration, adding new fields and enums in the pr… #1362

Merged
merged 5 commits into from
May 7, 2020

Conversation

auturgy
Copy link
Collaborator

@auturgy auturgy commented Apr 29, 2020

As discussed in dev call

@dogmaphobic Dev call discussed this and created PR.

Main points:

  • Need to make sure that extensions default to 0 for zero truncation. Also that if this is sent by a system that does not know about the fields, this will set the modes to the values you have specified - is that appropriate? Normally you'd have a 0 value which says - don't change the mode or whatever.
  • The interaction is that you can send MAV_CMD_REQUEST_WIFI_CONFIG to get the WIFI_CONFIG_AP:
    • Should it send back the password? If you do this then anyone who sends the message gets whatever the person set, which might be their phone number or whatever. Perhaps if emitting this message it should leave password empty or a hash?
    • Use MAV_CMD_REQUEST_MESSAGE and id for WIFI_CONFIG_AP - ie delete MAV_CMD_REQUEST_WIFI_CONFIG
  • Similarly, Jonas seemed to think that WIFI_CONFIG_AP being received should result in it being emitted again (as an acknowledgement)
    • If this is the case, it should be documented
    • Again, the password should not really be emitted ?
    • This is a broadcast message so will be processed by everyone. SHould it be? I guess only a wifi can act on it - everything else will just ignore, and that you'll only have one Wifi set up in a MAVLink network?

@dogmaphobic
Copy link
Contributor

  • Perhaps if emitting this message it should leave password empty or a hash?

It makes sense to send a hash only. We just need to agree on what hash type to use.

Also that if this is sent by a system that does not know about the fields,

Yes, I guess it would be safer to define something like:

 <entry value="0" name="WIFI_CONFIG_AP_MODE_UNDEFINED">

and

<entry value="0" name="WIFI_CONFIG_AP_RESPONSE_UNDEFINED">

That would indicate this message comes from an older version, which does not know about the new fields.

  • Use MAV_CMD_REQUEST_MESSAGE and id for WIFI_CONFIG_AP - ie delete MAV_CMD_REQUEST_WIFI_CONFIG

Yup, it makes sense, especially given that this is a new command.

With that said, I just need to know what's the final conclusion so it can be implemented.

@hamishwillee
Copy link
Collaborator

@dogmaphobic Final conclusion on what? If you just mean the hash, then we'd be led by what you suggest.

Otherwise, if you update the PR as discussed/you indicated I'd be good with it.

@dogmaphobic
Copy link
Contributor

dogmaphobic commented May 2, 2020

I think it's all there.

  • Added WIFI_CONFIG_AP_MODE_UNDEFINED and WIFI_CONFIG_AP_RESPONSE_UNDEFINED to handle old code (pre extension).
  • Added documentation of using MD5 hash for password when message is sent back as a response.
  • Removed the MAV_CMD_REQUEST_WIFI_CONFIG command in lieu of MAV_CMD_REQUEST_MESSAGE.

@hamishwillee
Copy link
Collaborator

@dogmaphobic OK, I've just added info about when this is emitted and I'll merge now. No need to complicate things if this works as you indicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants