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

Switch Nci property type as string. JB#61312 #6

Merged
merged 3 commits into from Mar 19, 2024
Merged

Switch Nci property type as string. JB#61312 #6

merged 3 commits into from Mar 19, 2024

Conversation

pvuorela
Copy link
Contributor

Main commit:

The qint64 type was problematic:
- Doesn't work in QML
- The Invalid64 on enum doesn't work without declaring the
enum as 64 bit. Which again might not be the best idea for qml.
- App developer needs to pay close attention whether to use
32-bit or 64-bit invalid value.

As this isn't getting used in the wild yet, let's just switch
the nci property as string for now. If there are c++ needs, a
separate getter can be added later.

The general 64-bit storage just because of one wider property, now
needing more special handling, felt heavy. Switched back to 32-bit and
using separate property for nci. Wouldn't expect new radio
technologies with new properties too soon.

In addition couple small cleanup commits.

As side-note the existing change signalling here feels excessive with separate signal for each property plus one generic propertyChanged() for int properties. If a cell changes, could assume all the cell properties changing. Only signal strength and that kind of details probably change independently.

Disclaimer: don't have NR to test this with.

The qint64 type was problematic:
- Doesn't work in QML
- The Invalid64 on enum doesn't work without declaring the
enum as 64 bit. Which again might not be the best idea for qml.
- App developer needs to pay close attention whether to use
32-bit or 64-bit invalid value.

As this isn't getting used in the wild yet, let's just switch
the nci property as string for now. If there are c++ needs, a
separate getter can be added later.

The general 64-bit storage just because of one wider property, now
needing more special handling, felt heavy. Switched back to 32-bit and
using separate property for nci. Wouldn't expect new radio
technologies with new properties too soon.
@pvuorela
Copy link
Contributor Author

Eh, for crying out loud, even the existing InvalidValue doesn't work in QML when used with == operator to compare values. Javascript uses float numbers and that big ones need fuzzy comparison.

@pvuorela pvuorela merged commit 890a214 into master Mar 19, 2024
@pvuorela pvuorela deleted the nci_api branch March 19, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants