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.

Sign up for GitHub

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

Jump to bottom

ADC: replace ioctl with uorb message #14087

Merged
merged 43 commits into from
Mar 20, 2020
Merged

ADC: replace ioctl with uorb message #14087

merged 43 commits into from
Mar 20, 2020

Conversation

SalimTerryLi
Copy link
Contributor

This is spitted from #13833 .

This PR focus on replace ioctls with uORB message. Some existing modules/drivers are affected.

@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 371ed98 to c6f9b61 Compare February 3, 2020 06:56
@SalimTerryLi SalimTerryLi requested a review from dagar February 3, 2020 06:56
@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 3, 2020 •
edited
Loading

Device ID is set but not used. All logic kept same as before. This should be safe to merge.

@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 3, 2020 •
edited
Loading

There are two things to be checked:

  1. whether old ioctl message manages channel_id field correctly? I mean that adc_msg[A].am_channel == A should be true if that channel is valid.
  2. Do all boards have the same 3v3 ADC vref? If not, there should be some macro defined cases in ADC.cpp.

Confirmed. There is no problem.

@SalimTerryLi SalimTerryLi marked this pull request as ready for review February 3, 2020 07:16
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from c6f9b61 to 1c9451e Compare February 6, 2020 05:12
bkueng
bkueng reviewed Feb 7, 2020
View reviewed changes
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Good rework, thanks.
What is the testing you have done?

src/drivers/adc/ADC.cpp Outdated Show resolved Hide resolved
src/modules/battery_status/analog_battery.cpp Show resolved Hide resolved
src/modules/battery_status/analog_battery.cpp Show resolved Hide resolved
src/drivers/rc_input/RCInput.cpp Outdated Show resolved Hide resolved
src/modules/battery_status/battery_status.cpp Outdated Show resolved Hide resolved
msg/adc_report.msg Outdated Show resolved Hide resolved
src/modules/battery_status/battery_status.cpp Outdated Show resolved Hide resolved
src/modules/battery_status/battery_status.cpp Outdated Show resolved Hide resolved
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 1126845 to b445912 Compare February 7, 2020 15:44
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 9807abc to 9188c4a Compare February 7, 2020 17:43
@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 8, 2020 •
edited
Loading

This is the last reference to that parameter. But I cannot find anything in the code which provides ADC specification such as sample precision and voltage reference. 0.0008 mostly indicate that the board is using 3v3 as vref with 2^12 bits precision but I cannot find anything related in code.
@bkueng Sorry but I'm unfamiliar with these devices. Any help is welcome.

@SalimTerryLi
Copy link
Contributor Author

Good rework, thanks.
What is the testing you have done?

Currently I have no hardware to test. This pr is diverged from a previous one so my test code on my platform could not be uploaded. If needed, I could commit them temporarily and then revert them for a clean PR.

@weekly-digest weekly-digest bot mentioned this pull request Feb 9, 2020
Closed
@bkueng
Copy link
Member

bkueng commented Feb 10, 2020

This is the last reference to that parameter. But I cannot find anything in the code which provides ADC specification such as sample precision and voltage reference. 0.0008 mostly indicate that the board is using 3v3 as vref with 2^12 bits precision but I cannot find anything related in code.
@bkueng Sorry but I'm unfamiliar with these devices. Any help is welcome.

They should not have been added to that board, you can just remove them from there (the default should be used).

@SalimTerryLi
Copy link
Contributor Author

This is the last reference to that parameter. But I cannot find anything in the code which provides ADC specification such as sample precision and voltage reference. 0.0008 mostly indicate that the board is using 3v3 as vref with 2^12 bits precision but I cannot find anything related in code.
@bkueng Sorry but I'm unfamiliar with these devices. Any help is welcome.

They should not have been added to that board, you can just remove them from there (the default should be used).

Acknowledged. I'm contacting dagar to get ADC reference voltage properly provided for each board.

@SalimTerryLi SalimTerryLi mentioned this pull request Feb 10, 2020
Merged
@weekly-digest weekly-digest bot mentioned this pull request Feb 16, 2020
Closed
davids5
davids5 reviewed Feb 21, 2020
View reviewed changes
src/drivers/adc/ADC.cpp Outdated Show resolved Hide resolved
src/drivers/rc_input/RCInput.cpp Show resolved Hide resolved
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from 88e63e3 to a14a1d0 Compare February 21, 2020 15:52
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from f6eae80 to da0a336 Compare February 22, 2020 07:58
@SalimTerryLi
Copy link
Contributor Author

SalimTerryLi commented Feb 22, 2020 •
edited
Loading

Solved conflict with master 4c1adc0
Probably caused by in-formatted white spaces that make format doesn't deal with.

@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch 3 times, most recently from c4d7c6b to 882acd8 Compare February 22, 2020 08:13
@SalimTerryLi
Copy link
Contributor Author

It's ready. @dagar
If someone could provide a test on the real hardware to prove its correctness would be better. Sorry for that I do not have any device to do such a test. If analog battery is monitored correctly, then this is safe to merge.

davids5
davids5 reviewed Feb 22, 2020
View reviewed changes
src/drivers/adc/ADC.cpp Outdated Show resolved Hide resolved
src/drivers/rc_input/RCInput.cpp Show resolved Hide resolved
@weekly-digest weekly-digest bot mentioned this pull request Feb 23, 2020
Closed
SalimTerryLi and others added 20 commits March 20, 2020 17:09
@SalimTerryLi
purge BOARD_ADC_POS_REF_V_FOR_*_CHAN
735dceb
@SalimTerryLi
update AEROFC_ADC
33692e0 
1. remove cdev implement and use uORB publication
2. publish all channels instead of only battery voltage moniting channel
3. correct vref to 3.0f
4. temporarily disable test function
@SalimTerryLi
remove comments.
d17e94f
@SalimTerryLi
Fix logic on batt assignment: PX4#14087 (comment)
894900a 
This would imporve the performance
@SalimTerryLi
force fill unused channel id to -1
20fa835
@SalimTerryLi @davids5
Update src/modules/battery_status/battery_status.cpp
7945a5e 
Critical bug fix 1

Co-Authored-By: David Sidrane <[email protected]>
@SalimTerryLi @davids5
Update src/modules/battery_status/battery_status.cpp
cc473d9 
Critical bug fix 2

Co-Authored-By: David Sidrane <[email protected]>
@SalimTerryLi
fix param update: i_channel
7e15d73
@SalimTerryLi
remove unused declaration
eacf403
@SalimTerryLi @bkueng
Update msg/adc_report.msg
806f6c2 
Co-Authored-By: Beat Küng <[email protected]>
@SalimTerryLi @bkueng
Update src/drivers/rc_input/RCInput.cpp
38318bf 
Critical bug fix

Co-Authored-By: Beat Küng <[email protected]>
@SalimTerryLi
update bbblue parameter set in the startup script
e968c1a
@SalimTerryLi
auto-generated value of PX4_MAX_ADC_CHANNELS (run time)
cd98e4a
@SalimTerryLi
remove const_cast
95da375
@SalimTerryLi
make adc and jig voltages tests from systemcmds/tests using adc_report
a40966d
@SalimTerryLi
introduce ADC_DP_V_DIV
50e8f60 
analog differential pressure sensor reads from uORB(sensors.cpp)
@SalimTerryLi
using arraySize()
0531c8a
@SalimTerryLi
remove comments
a4ab180
@SalimTerryLi
cleanup _adc_fd in sensors.cpp
e404591
@SalimTerryLi
purge CDev implementation of drivers/adc
b431f72
@SalimTerryLi SalimTerryLi force-pushed the pr-adc_replace_ioctl_with_uorb-improve branch from ba94c6a to b431f72 Compare March 20, 2020 09:17
@SalimTerryLi SalimTerryLi requested a review from bkueng March 20, 2020 09:40
@bkueng bkueng merged commit dc8e775 into PX4:master Mar 20, 2020
@SalimTerryLi SalimTerryLi deleted the pr-adc_replace_ioctl_with_uorb-improve branch April 24, 2023 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkueng bkueng bkueng approved these changes

@davids5 davids5 davids5 left review comments

@dagar dagar Awaiting requested review from dagar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@SalimTerryLi @bkueng @jinger26 @dannyfpv @jorge789 @Junkim3DR @mrpollo @davids5 @dagar