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

Continue cleanup work #27

Merged
merged 3 commits into from
Jan 28, 2025
Merged

Continue cleanup work #27

merged 3 commits into from
Jan 28, 2025

Conversation

bellegarde-c
Copy link
Member

2a7bde6826a8a755c1f6adc6df2a43b9d8374420 mm_modem_simple: Use ModemManagerState and ModemManagerAccessTechnology
ee2132eb559653b765c8378fd2564bc97963dcdf mm_types: Add ModemManagerPortType
f7ac4e54ab1ea888d42e565223b4267249499b58 mm_modem: Move common types to own file

There is something strange in this commits:

In ee2132e, MM_MODEM_PORT_TYPE_AT value was wrong, 2 instead of 3: https://github.com/linux-mobile-broadband/ModemManager/blob/main/include/ModemManager-enums.h#L669

In 2a7bde6, all modem state values were wrong:

  • Enabled is 6, not 7
  • Searching is 7 not 8
  • Registered is 8, not 9

I'm testing this for some days, for now it works...

Copy link
Member

@g7 g7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look fine to me, and long overdue, thanks! Unsure why the values where wrong in the first place, but I'm not an expert in ofono2mm in general...

I think we can either merge this straight away or at least injecting it in droidian-release so that we can have some wider testing in 101, what do you think?

@bellegarde-c
Copy link
Member Author

Using this on my main phone since 1 month, works OK so yes, looks safe to push it to droidian-release.

g7 added a commit to droidian/droidian-release that referenced this pull request Jan 13, 2025
See for more details, droidian/oFono2MM#27

Injecting here for broader testing on 101

Signed-off-by: Eugenio Paolantonio (g7) <[email protected]>
@g7
Copy link
Member

g7 commented Jan 26, 2025

I think we can merge this, what do you think?

@bellegarde-c
Copy link
Member Author

Yes, works great for me.

@g7
Copy link
Member

g7 commented Jan 28, 2025

Great, merging, thanks!

@g7 g7 merged commit d848419 into droidian Jan 28, 2025
2 checks passed
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.

2 participants